Constructing properties, connectors, inputs and outputs and their documentation

[aseth1]

The API for specifying these key attributes is still tricky and not consistently (automatically) documented. Properties with macros do appear in the header and their description is present so that is nice, but even for properties it requires calling specialized construct... methods in the components constructor. This requirement leads to general problems of enforcement (not part of an enforceable interface) and lacks a required presence (and documentation) in the header. Inputs and Outputs are particularly useless if no one knows they are there. It should appear automatically in Doxygen and in a prominent way. I think this will be critical for scripting to take-off.

The goal should be to list properties, connectors, inputs and outputs as distinct sections (tables) in the header and have everything else about their construction be handled automatically.

The question is how to achieve this and at what expense?

Two approaches so far: 1) Statically define the contents of corresponding property, connector, ... tables (as strings, or structs) in the header and have the constructor initialize the corresponding table (instance) members from this template. 2) Apply/develop more macro magic to define each attribute and auto implement private virtual constructProperties, constructConnectors, ... which are invoked by base constructInfrastructure() and apply a discipline to call constructInfrastructure().

@sherm1, @aymanhab, @chrisdembia. @msdemers please add your ideas/concerns. Thanks.

[chrisdembia]

For reference, I think this is the ideal interface for component developers (this example is just illustrative):

class OSIMACTUATORS_API CoordinateActuator : public Actuator {
OpenSim_DECLARE_CONCRETE_CLASS(CoordinateActuator, Actuator);
public:
OpenSim_DECLARE_PROPERTY(optimal_force, double, "The max force...", 1);
// gives get_optimal_force();, set_optimal_force();

OpenSim_DECLARE_CONNECTOR(coordinate, Coordinate, "The coordinate that this joint actuates.");
// gives get_coordinate(); set_coordinate();

OpenSim_DECLARE_OUTPUT(generalized_force, double, &CoordinateActuator::calcForce, "Force or torque applied along the coordinate.");
// gives get_generalized_force_output();, get_generalized_force_value();

OpenSim_DECLARE_INPUT(control_signal, double, SimTK::Stage::Model, "Description...");
// gives get_control_signal_input(); set_control_signal_input(); get_control_signal_value().
};

Potential problems.

1. We discussed having a "default value" field in the macro instead of calling constructPropety_optimal_force() manually, but there are cases where this default value is not calculated in a simple one-expression way (e.g., Array_<double> v(2); v[0] = -1; v[1] = 1;). See here for an example; this isn't the best example but I've seen others...just can't find them now.
2. Could forcing the construction in the header lead to impossible cases where a circular reference could not be resolved (e.g., the constructProperty call must be in the cpp. Say, a component that has an internal Model property).

I think avoiding the manual constructProperty_<name>() is tempting, but sounds very difficult and may not be possible for all cases. If I think of how much gruntwork the macro is already doing for me (allowing serialization, etc.), having to call constructProperty_<name>() seems like a small cost to pay, even though it's often not ideal.

[chrisdembia]

Here's some reading from David Cheriton's software engineering class at Stanford. Actually probably not too useful, but I wanted to share it.

Here's a chapter on inter-object relationships. Not actually as useful as I thought it would be (just skimmed it): https://web.stanford.edu/class/cs249a/coursereader/ch9.pdf

Representing a hierarchy like a filesystem ("mounting"): https://web.stanford.edu/class/cs249a/coursereader/ch8.pdf

[sherm1]

We discussed having a "default value" field in the macro ... but there are cases where this default value is not calculated in a simple one-expression way

I think we could deal with the default value problem. We could add some convenience constructors to deal with common cases (e.g. to Array), and for complex situations the user could probably provide a private-method call that returns the value to be used as the default.

having to call constructProperty_<name>() seems like a small cost to pay

I didn't like doing it that way but we couldn't figure out anything better. I agree that it isn't that awful but if there is something similar a user has to do for connectors, inputs, and outputs as well as properties that is probably too much bookkeeping for most people to get right.

[sherm1]

One possible fallback if we can't figure out an ideal solution is to make sure there is a runtime error issued if any of the mother-may-I steps are left out. I think that is currently the case for missing constructProperty() calls though I'm not certain.

[sherm1]

For reference, I think this is the ideal interface for component developers ... nice example above

I agree. It may not be the best of all possible worlds but it is very consistent and IMO meets the "principle of least astonishment" test. We know from experience that the existing object and property macros are workable; they produce homely but serviceable doxygen; and require no infrastructure work and ongoing maintenance on our part (like a code preprocessor could). But it would be very nice if those macros were the end of the story and the user didn't have to follow any other mysterious incantations.

[chrisdembia]

they produce homely but serviceable doxygen

This is somewhat off-topic, but I think I have come up with an unhomely doxygen solution for the properties, but it's not ready to reveal yet (see here). Here's an example for the Millard muscle:

So, I am confident we can achieve unhomely doxygen with the input, output, and connector macros.

[sherm1]

The goal should be to list properties, connectors, inputs and outputs as distinct sections (tables) in the header and have everything else about their construction be handled automatically. The question is how to achieve this and at what expense?

A few random thoughts before they escape:

• c++11 has generalized initializer support that was missing before. I haven't used those yet but it is conceivable that could add the functionality we need to have the declaration macros cause something to get done at construction
• Another possibility that comes to mind is to have each macro create a local class and a data member of that class type, with the default constructor of the local class designed to perform the needed runtime construction. I believe we thought of this before and there was a fatal flaw to the idea which escapes me at the moment. However it is possible that it might be done now in c++11.

[sherm1]

This is somewhat off-topic, but I think I have come up with an unhomely doxygen solution

Nice! That's a great idea. BTW, to get those /** @{ **/ things to work you need to precede them with a line that looks something like /** @name Section you want them to show up in **/

[chrisdembia]

Nice! That's a great idea. BTW, to get those /\ @{ / things to work you need to precede them with a line that looks something like / @name Section you want them to show up in **/

Thanks, but I think I tried that and ran into issues b/c you aren't supposed to actually be able to nest groups.

[aymanhab]

A couple random thoughts and concerns: @chrisdembia . "Could forcing the construction in the header lead to impossible cases where a circular reference could not be resolved (e.g., the constructProperty call must be in the cpp. Say, a component that has an internal Model property"

• Indeed, this is a signficant problem already, any object-type used in a property must be "included" rather than forward declared in headers, I'm fighting it while doing geometry and it leads to very fragile include(s).

Before we go too much into macros we must make sure that we're not painting ourselves into a corner by having the macro be the only way to add Properties/Inputs/Outputs as these macros are not accessible from scripting and I'd hate that a user who wants to add an output is forced into using C++. So, at least make it "possible" to add outputs from scripting.

[aseth1]

Great discussion fellas. @aymanhab you raise an important question: do we want to support component building via scripting? Although the construct methods exist in the API they are intended to be called in the constructor and protected so not accessible via scripting. I fear the integrity of a component could be lost if scripting or API users could change the properties, connector, inputs and outputs via scripting. Being able to create new "classes" of components via scripting would be very powerful, however. If the scripting language has the concept of a class or defining a object type it would be great to expose the construct methods in this context of deriving from and creating a new Component. When a C++ class is wrapped in Python can it be subclassed? In that case can the protected method of the class be accessed (if wrapped)?

[aymanhab]

@aseth1 Python has the concept of classes and there was a recent post on the forum https://simtk.org/forums/viewtopic.php?f=91&t=4829&p=12523&hilit=ForceReporter#p12523 from a user who converted our tutorials into python scripts but failed to create his own ForceReporter (which would have been awesome since it would have given him access to large tool-chest in python). While properties may not be very important for scripting users, extending classes to provide new functionality or to add outputs would definitely be very useful.

[aseth1]

@aymanhab and they would do this by writing a new subclass in Python? That would be pretty cool. If so you definitely would want the ability to add properties to capture parameters/settings of your new capabilities. Can you wrap protected methods so they can be i) called by the derived class only and ii) can override virtual methods in Python? How are (pure) virtual methods exposed?

[chrisdembia]

Here's how you get "cross language polymorphism" with SWIG: http://www.swig.org/Doc2.0/Python.html#Python_directors

Right now, only a few classes have this enabled (see here). We can enable it for all classes using %feature("director") somewhere in that file, but the swig documentation says we should be selective since there is overhead.

i) In python, there is no such thing as private. There is only a convention that methods starting with an underscore are "hidden" and and are meant for internal use, but anyone can still use them. However, the protected method would not have this underscore and would be exposed. However, I just checked, and the current swig wrapping does not expose protected or private methods. ii) yes, you should be able to override methods.

[aymanhab]

Here's a version of the script from user forum that works In trunk environment. Had to modify Manager class to create integrator intenally due to #156 ============ Relevant part =================

......
class MyForceReporter(opensim.AnalysisWrapper):
    def begin(self, *args):
        print "begin message from"+self.getName()
        return 0

    def step(self, *args):
        print "step message"
        return 0

    def end(self, *args):
        print "end message"
        return 0

reporter1 = MyForceReporter(osimModel)
reporter1.setName("ReporterFromPython")
osimModel.addAnalysis(reporter1)
........

results in printing of

Integrating from 0.0 to 4.0
begin message fromReporterFromPython
step message
step message
step message
step message
step message
step message
end message

to python console.

[chrisdembia]

@msdemers is right: in c++11, a constructor can call another constructor in the same class ("delegating constructors"). As Matt mentioned, we could define default constructors in the OpenSim_DECLARE_CLASS macros, and this constructor can construct properties. Then, convenience constructors would need to call the default constructor.

Also, here's what @sherm1 was saying about c++11 member initialization: http://stackoverflow.com/questions/12170160/c-initialization-of-member-variables, http://www.stroustrup.com/C++11FAQ.html#member-init. This would be a really slick solution to our problem, so long as the initializer can be a member function call (see here).

[chrisdembia]

@aymanhab that's so cool! How about protected members?

[chrisdembia]

In-class member initializers can call methods:

This code:

#include <iostream>                                                             
class A {                                                                       
protected:                                                                      
    int init(int i) {                                                           
        return i;                                                               
    }                                                                           
};                                                                              

class B : public A {                                                            
public:                                                                         
    int j = init(5);                                                            
};                                                                              

int main() {                                                                    
    B b;                                                                        
    std::cout << b.j << std::endl;                                              
}

produces 5.

I think this solves the problem of users calling constructProperty_<name> themselves!

[aymanhab]

@chrisdembia Protected and private methods/variables are not exposed by default. By introducing the intermediate class AnalysisWrapper (which is exposed using the swig/director feature) I was able to customize the interface of Analysis to fit my needs. I see we may need to expose protected methods but I see no reason whatsoever to do private methods or data members.

[sherm1]

I think this solves the problem of users calling constructProperty_ themselves!

That looks like a winner, Chris!

[sherm1]

in c++11, a constructor can call another constructor in the same class ("delegating constructors")

That worked in c++03 also; Simbody uses it a lot. The syntax was awkward though. Here is an example. That construct is called "placement new".

[chrisdembia]

That looks like a winner, Chris!

Thanks for the insight Sherm. Here's a branch with this partially implemented: https://github.com/opensim-org/opensim-core/tree/default-property-macro

[aseth1]

Gentlemen, I think I get something like this to work for Outputs.

class Foo : public Component {
    OpenSim_DECLARE_CONCRETE_OBJECT(Foo, Component);

public:
    /** Define Component Outputs */
    Output<double> out1{ "Output1", &Foo::getSomething, this, SimTK::Stage::Time };
    Output<SimTK::Vec3> out2{ "Output2", &Foo::calcSomething, this, SimTK::Stage::Time };
    Output<SpatialVec> out3{"BodyAcc", &Foo::calcSpatialAcc, this, SimTK::Stage::Velocity};

I believe Output is the trickiest because it has to bind to a member function. I need this to inform the bind of the actual Component and add the Output to the component's table of outputs. If you have any ideas how to access the Component without this or to hide the fact that we are doing it, I would love to know about it!

[chrisdembia]

Create a member function in Component that returns the Output? I think you mentioned this idea to me. Is there a reason you want to avoid that?

[aseth1]

@chrisdembia I thought it would be cleaner to just define the Output as a data member. Calling constructOutput would also require macros to translate the Output settings to the construct method.

I'm also not convinced about the usefulness of automatically providing more access methods, e.g. from your example:

OpenSim_DECLARE_OUTPUT(generalized_force, double, &CoordinateActuator::calcForce, "Force or torque applied along the coordinate.");
// gives get_generalized_force_output();, get_generalized_force_value();

OpenSim_DECLARE_INPUT(control_signal, double, SimTK::Stage::Model, "Description...");
// gives get_control_signal_input(); set_control_signal_input(); get_control_signal_value().
};

seems it would just bloat components especially those with many outputs. Consider that the Output is a wrapper for a member function already, it seems rather redundant and unnecessary to then provide an accessor method to that Output.

I think other Components like reporters should be able to get a handle on an Output (e.g. const Output&) by name and then hang on to it for the duration of the system. An Input formalizes that requirement to have an Output handle.

I vote against automatically adding accessor methods. It is convenient/useful for properties. It might make sense for Connectors but then you's also want a get_<connected_component>_is_connected() too, right? I am inclined to leave them out, for now.

[chrisdembia]

Okay I see your point. I now agree with you about outputs. I now realize that the only time you should be getting an Output is when (1) you wire it into an input, or (2) you report its value in a reporter.

Following along with (1) above, though, I think it may make sense to have automatically generated accessors for inputs. In my own component, I'd rather not make repeated calls to getInputValue(string), knowing that this will cause a string look-up. Holding onto a reference to the Input is an improvement, but I still need to get that reference at the top of my method, which is being called throughout integration.

Calling constructOutput would also require macros to translate the Output settings to the construct method.

I don't understand. Also, I'm still suggesting using a data member. Just suggesting using a method to initialize that data member (in a method, you can access the this pointer). Without a macro, will it still be possible to have documentation in doxygen and in the XML file for each connector?

[aseth1]

If Inputs and Outputs are members, then we could: 1. make groups to list them in doxygen, 2. access the members internally (we could make them protected- another thing I don't like is that properties are listed as public) without any getters. I understand that having accessors to members is preferred, but the rationale for that is to hide the underlying data model. For inputs and outputs, these are specific object to encapsulate data and access, so direct internal access to them makes a lot of sense (to me at least) and not to mention it is easy for the user to use and understand. I list Inputs and Outputs members , I talk to those members to get values i need internally and the wiring to the rest of the model/system is handled automatically. I think this would be ideal.

[sherm1]

I understand that having accessors to members is preferred, but the rationale for that is to hide the underlying data model.

That's not the only reason. One that applies in the case of Properties is the need to know when someone modifies one, so we can set the dirty bit. I don't know if there is anything analogous for Inputs and Outputs, but it's worth thinking about because once you let the direct-access cat out of the bag, it can't be put back in!

[chrisdembia]

Using the data members directly instead of through getters sounds fine; I now see that this avoids using string lookups :smile:. In response to @sherm1 's comment, that dirty stuff can maybe be handled within the Input/Output object.

1. make groups to list them in doxygen,

A quick google search showed that doxygen does document public member variables; do you know if it documents protected ones?

[aseth1]

@chrisdembia @sherm1 , yes Input/Output already answer questions about their internal status: e.g. isConnected(). Dirtying the Component when an Input changes makes sense, but the Input itself performs that service, albeit it in an indirect way since the underlying property serializing the connector is modified, but I think that is fine. The definition of whether a component (object) has changed is whether or not one of its properties has changed hold up.

[jenhicks]

85 and #86

[chrisdembia]

Related: #574

[chrisdembia]

Macro for outputs: #806 Macro for inputs: #815

Macros for connectors are more troublesome and run into issues of forward declaration; see this branch for a first attempt: https://github.com/opensim-org/opensim-core/tree/in-class-connectors

Sherm's idea of using a nested class might help here:

Another possibility that comes to mind is to have each macro create a local class and a data member of that class type, with the default constructor of the local class designed to perform the needed runtime construction. I believe we thought of this before and there was a fatal flaw to the idea which escapes me at the moment. However it is possible that it might be done now in c++11.