Uploaded image for project: 'Data Management'
  1. Data Management
  2. DM-1461

C++ Redesign -- Result definition for custom algorithms

    XMLWordPrintable

    Details

      Description

      Additions to Jim's redesign to make it easier to define custom results.

        Attachments

          Issue Links

            Activity

            Hide
            pgee Perry Gee added a comment -

            This is a prototype of a ResultKey for a Result object which uses macros and generic routines. The code specific to the Result (in this case SdssShapeResult) is in the macros. (MAKE_RESULT_FIELD and INIT_RESULT_FIELDS3)

            This prototype is not really completely done, but it is enough to demonstrate the concept.

            Some limitations on this implementation:

            1. I removed the scalar members of SdssShape because the code assumes that all of the members of the Result object have been defined in Results.h (essentially as what we called components in the past). I need to define a second "MAKE" macro to make this work with scalars.
            2. I for some reason was not able to make macros with omitted arguments work, so I had to change FluxUtilities to add an UNCERTAINTY argument. Probably not necessary.
            3. I never got the static initialization of my function arrays to work, so I was forced to add an init() method to initialize these arrays as instance variables. I think someone who knows C++ better than I do could fix this.

            Show
            pgee Perry Gee added a comment - This is a prototype of a ResultKey for a Result object which uses macros and generic routines. The code specific to the Result (in this case SdssShapeResult) is in the macros. (MAKE_RESULT_FIELD and INIT_RESULT_FIELDS3) This prototype is not really completely done, but it is enough to demonstrate the concept. Some limitations on this implementation: 1. I removed the scalar members of SdssShape because the code assumes that all of the members of the Result object have been defined in Results.h (essentially as what we called components in the past). I need to define a second "MAKE" macro to make this work with scalars. 2. I for some reason was not able to make macros with omitted arguments work, so I had to change FluxUtilities to add an UNCERTAINTY argument. Probably not necessary. 3. I never got the static initialization of my function arrays to work, so I was forced to add an init() method to initialize these arrays as instance variables. I think someone who knows C++ better than I do could fix this.
            Hide
            pgee Perry Gee added a comment -

            I put another version of the prototype in u/pgee/DM-1461b.

            The difference is that the function arrays are in the Result object, which is probably more natural.

            The ResultKey could probably go back to being a FunctorKey in this implementation.

            Show
            pgee Perry Gee added a comment - I put another version of the prototype in u/pgee/ DM-1461 b. The difference is that the function arrays are in the Result object, which is probably more natural. The ResultKey could probably go back to being a FunctorKey in this implementation.
            Hide
            jbosch Jim Bosch added a comment -

            I've taken a look at this, and I think it's probably time to call it quits on this issue. You've gotten it far enough that I think we can see that it could be made to work with some more effort, but we can also see what the tradeoff of a complete solution would look like - it'd be ton of macros (to support all different possible numbers of subcomponents, support new scalar outputs that don't have predefined subcomponents, and just generally polish this up), and probably a bit of bloat in the result objects to hold all the extra function pointers.

            IMO, that tradeoff isn't worth it; I think the amount of hard-to-maintain macro code we'd end up putting in the framework would be larger than the amount of boilerplate we saved in all future plugins. I'm also worried that our potential reviewers (K-T and RHL in particular) are perhaps even more hostile to preprocessor macros than I am (that's why Source.h is generated using M4 rather than the preprocessor, for instance).

            Do you agree, or do you think there's more to be done here? I do appreciate your work on this, as I think it's important to be able to rule out designs like this one and the others you already looked into.

            Show
            jbosch Jim Bosch added a comment - I've taken a look at this, and I think it's probably time to call it quits on this issue. You've gotten it far enough that I think we can see that it could be made to work with some more effort, but we can also see what the tradeoff of a complete solution would look like - it'd be ton of macros (to support all different possible numbers of subcomponents, support new scalar outputs that don't have predefined subcomponents, and just generally polish this up), and probably a bit of bloat in the result objects to hold all the extra function pointers. IMO, that tradeoff isn't worth it; I think the amount of hard-to-maintain macro code we'd end up putting in the framework would be larger than the amount of boilerplate we saved in all future plugins. I'm also worried that our potential reviewers (K-T and RHL in particular) are perhaps even more hostile to preprocessor macros than I am (that's why Source.h is generated using M4 rather than the preprocessor, for instance). Do you agree, or do you think there's more to be done here? I do appreciate your work on this, as I think it's important to be able to rule out designs like this one and the others you already looked into.
            Hide
            pgee Perry Gee added a comment -

            I don't agree with Jim's last comment. It would not take more macros to do the implementation this way. It just takes one macro for components which have a predefined Result/ResultKey types defined, one macro for components which are scalar types, and one macro to define and initialize the function arrays. The class which stores these values can be made into a templated class which would be very easy to use without additional macro definition.

            However, Jim and I agree that a macro solution to the problem of creating custom outputs is not very desirable, and I don't believe it can be done in C++ either with straight C++ code or any kind of simple templated code.

            Show
            pgee Perry Gee added a comment - I don't agree with Jim's last comment. It would not take more macros to do the implementation this way. It just takes one macro for components which have a predefined Result/ResultKey types defined, one macro for components which are scalar types, and one macro to define and initialize the function arrays. The class which stores these values can be made into a templated class which would be very easy to use without additional macro definition. However, Jim and I agree that a macro solution to the problem of creating custom outputs is not very desirable, and I don't believe it can be done in C++ either with straight C++ code or any kind of simple templated code.
            Hide
            ktl Kian-Tat Lim added a comment -

            Agreed that I wasn't much liking the way this was turning out, but not so much over the macros. Rather, any time you're using function pointers, there's probably a better way.

            Show
            ktl Kian-Tat Lim added a comment - Agreed that I wasn't much liking the way this was turning out, but not so much over the macros. Rather, any time you're using function pointers, there's probably a better way.
            Hide
            pgee Perry Gee added a comment -

            I very much agree with KT's comment. As it turns out, the use of function pointers, as well as the unfortunately complicated INIT macro, are hiding a more basic problem, which is that the class design of the Result objects and FunctorKeys probably needed to be rethought to do this all more cleanly. Given that Jim had already designed and implemented most of this in his prototype, I wasn't willing to suggest a wholesale redesign. Probably should have bailed on this issue when I realized what was needed.

            Show
            pgee Perry Gee added a comment - I very much agree with KT's comment. As it turns out, the use of function pointers, as well as the unfortunately complicated INIT macro, are hiding a more basic problem, which is that the class design of the Result objects and FunctorKeys probably needed to be rethought to do this all more cleanly. Given that Jim had already designed and implemented most of this in his prototype, I wasn't willing to suggest a wholesale redesign. Probably should have bailed on this issue when I realized what was needed.
            Hide
            pgee Perry Gee added a comment -

            I should probably have been more explicit in my last comment. I think that in order to avoid the whole issue of function pointer arrays, a Result object needs to be made up of objects which all descend from a common class, and that class needs to have the logic to get, set and add fields so that the function pointer array isn't needed to make the Components appear to be of a single type.

            This all would have be different in a significant way from what Jim has already designed.

            Show
            pgee Perry Gee added a comment - I should probably have been more explicit in my last comment. I think that in order to avoid the whole issue of function pointer arrays, a Result object needs to be made up of objects which all descend from a common class, and that class needs to have the logic to get, set and add fields so that the function pointer array isn't needed to make the Components appear to be of a single type. This all would have be different in a significant way from what Jim has already designed.

              People

              Assignee:
              pgee Perry Gee
              Reporter:
              pgee Perry Gee
              Reviewers:
              Jim Bosch
              Watchers:
              Jim Bosch, Kian-Tat Lim, Perry Gee
              Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved:

                  Jenkins

                  No builds found.