OpenSim API and its Documentation

[mitkof6]

Dear developers,

It is not really an issue, but some suggestions regarding the API and the documentation. The OpenSim framework is really a great tool, with a lot of potential. I am using the tools that you developed quite a lot time, and have come a long way in understanding a lot of things about how to build new features and to set up simulations. Unfortunately, most of the time I spend in developing is to find a way to bypass something or to do something that is supposed to be an easy task. For example, how to set the input of the muscle excitations between the iterations of the integration. The first thing that I do is to check the API, and to find an interface function, but at the end, I have to look into the source code, or at GitHub for test code so I can understand how to use them. Clearly, the documentation is poor and there are functions that I can’t understand how to use. For example the setControls of Model class, so I was forced to develop a custom controller, knowing that there must be a way to set the muscle excitations (by the way also checked the setExcitation function of Muscle class). My suggestion is that the end user of the API must see only the interface and not to look into the source code and to be able to understand how to use it by reading the documentation. Many of the functions have pre-requirements (e.g. the topology must be realized) and if the user is not well familiar with the internal process will have run-time or compile errors.

I am willing to help in documenting and revising the source code, so please let me know how I can assist you.

Regards,
Stanev

[chrisdembia]

+10

[aseth1]

@mitkof6 thank you for the feedback and willingness to help. We generally need more input on how developers like you want to use the API. @chrisdembia has setup a Sandbox (opensim-core/OpenSim/Sandbox) for use cases with future code and pseudo-code. You can submit PRs for code (including MATLAB and Python scripts). We encourage you to contribute your use cases.

For the specific methods you mentioned, it would be extremely valuable for us to see how you were using Model::setControls() and why it didn't work as expected. Same for Muscle::setExcitation(). If you could post the code (or snippets) here we can review/discuss.

Thank you so much for helping us!

[chrisdembia]

We're also interested in your more general ideas for improving our documentation. We are planning to write some new "getting started" docs for the API, and we're also thinking of writing a 2 page reference sheet. How much of the problem do you think is poor documentation versus hard-to-understand and unclear class hierarchy/designs?

[mitkof6]

@aseth1 you are welcome. I didn't knew about Sandbox (very good idea, you should add a reference in the readme). About setControls, I am sure that I miss something, but I can't understand what. I have checked the github for references, but can't figure it out.

#include <iostream>
#include <string>
#include <random>

#include <OpenSim\OpenSim.h>

using namespace OpenSim;
using namespace SimTK;
using namespace std;


/**
* Main function
*/
int main()
{

    try {

        Model model("model path ");

        // kinematics
        Kinematics* kinematics = new Kinematics();
        kinematics->setModel(model);
        model.addAnalysis(kinematics);

        // initialize
        model.buildSystem();
        State& state = model.initializeState();


        // manager
        RungeKuttaMersonIntegrator integrator(model.getMultibodySystem());
        Manager manager(model, integrator);


        // integrate

        manager.setInitialTime(0);
        manager.setFinalTime(1);
        manager.integrate(state);

        cout << "Model has " << model.getNumControls() << " controls" << endl;

        // set controls

        Vector controls(2, 0.0);
        controls[0] = 0;
        controls[1] = 1;
        cout << "Controls " << controls << endl;
        // I am asked to realize position so I can set the controls
        // does't manager already realized it?
        model.realizePosition(state); 
        model.setControls(state, controls); 
        //model.realizeDynamics(state);// should I realize dynamics after setting the cotrolls?

        // same as above, I have the interface that I can set the excitation
        // but nothing happens
        model.updMuscles().get("rect_fem_r").setExcitation(state, controls[1]);

        // integrate again

        manager.setInitialTime(1);
        manager.setFinalTime(2);
        manager.integrate(state);


        // store results

        kinematics->printResults(model.getName(), "path");

        Storage stateStorage(manager.getStateStorage());
        model.updSimbodyEngine().convertRadiansToDegrees(stateStorage);
        stateStorage.print("path");


    }
    catch (const std::exception& ex)
    {
        std::cout << "Exception: " << ex.what() << std::endl;
    }
    catch (...)
    {
        std::cout << "Unrecognized exception " << std::endl;
    }

    system("pause");

    return 0;
}

By the way I had some issues with initSystem, buildSystem, initializeState. Fortunately, you added some macros to ensure proper use (Simbody). The documentation is clear, but don't you think you can automate the process.

Thank you for your time!

[mitkof6]

@chrisdembia I can pinpoint some aspects that I found difficult when I started, till now. I am not a software hierarchy/design expert, but I can write some thoughts about it. Also, I have been getting a lot of mails from API users with issues, asking me how to solve them. I encourage them to write in the foroum, so anyone can see the solution. I can gather all the mails and send you. Please send me an email (jimstanev@gmail.com) with more details about what exactly are you preparing so I can give you feedback.

Regards

[chrisdembia]

By the way I had some issues with initSystem, buildSystem, initializeState.

These are definitely confusing, and we need some way to clarify these methods or eliminate them. Usually, clients only need to use initSystem().

you should add a reference in the readme

We only added the directory last week. How about to the CONTRIBUTING.md? It seems like something only more advanced users/contributors would be interested in.

[chrisdembia]

Also, I have been getting a lot of mails from API users with issues, asking me how to solve them.

Is this because they see you on the forum? Thanks for forwarding them to the forum; that's helpful.

but I can write some thoughts about it.

That would be great! You can share those thoughts in these issues, or open new issues for specific topics if they are large and distinct. There might also be some exiting issues related to your thoughts.

Please send me an email (jimstanev@gmail.com) with more details about what exactly are you preparing

You mean about the 2 page reference, right?

#include <OpenSim\OpenSim.h>

Small comment, but for platform-independence this should use a forward slash.

[mitkof6]

We only added the directory last week. How about to the CONTRIBUTING.md? It seems like something only more advanced users/contributors would be interested in.

I think maybe the sandbox folder should be moved in the first directory of the project, so the new user can see the sample code. And it should be mentioned in the main readme, because the user may not check the contributing.md. I will write some examples and push them.

These are definitely confusing, and we need some way to clarify these methods or eliminate them. Usually, clients only need to use initSystem().

For example, if I want to initialize the Manager in the constructor, I need the mutlibody system, so I build the model. Then let's say that I make some more changes and before the integration I call initSystem, instead of initializeState to get the state. I think ether initSystem must not exist, or the build infrastructure must be more dynamical (e.g. to check which components are not initialized and to call the appropriate methods).

That would be great! You can share those thoughts in these issues, or open new issues for specific topics if they are large and distinct. There might also be some exiting issues related to your thoughts.

I will think about it and write you.

You mean about the 2 page reference, right?

Yes if you need something, send me an email.

Small comment, but for platform-independence this should use a forward slash.

Thank you, I will keep in mind (VS puts it automatically, maybe I should start working with emacs :)).

[chrisdembia]

I think maybe the sandbox folder should be moved in the first directory of the project, so the new user can see the sample code. And it should be mentioned in the main readme, because the user may not check the contributing.md. I will write some examples and push them.

Hmm... I hadn't thought about moving the folder to the root directory, and I'm not opposed to it. The Sandbox folder is not documentation for users; it's for developers and for advanced users who want to have a say as to what OpenSim should be able to do in the future. In fact, we initially named the folder Future. I don't want the folder to be too exposed to the new user since the code in there is likely not to function yet. Sample code is separate and is in the examples directory. Do you still think the Sandbox folder would be of interest to the new or typical user?

[chrisdembia]

or the build infrastructure must be more dynamical (e.g. to check which components are not initialized and to call the appropriate methods).

This would be awesome. We've discussed it in the past. For example, we've discussed having a callback system attached to the properties, so that every time a property changes, the property can call any of the functions you specify as callbacks. It might be fairly hard to implement, given the current code base.

[sherm1]

every time a property changes, the property can call any of the functions you specify as callbacks

I'm not sure if this is enough for your purposes, but there is already an automatically-maintained flag in every OpenSim Object called isObjectUpToDateWithProperties(). That flag is set to false whenever a Property is modified (see OpenSim::Object doxygen).

[chrisdembia]

Are you talking to me or @mitkof6? That flag is useful but the users of the property are still responsible for checking the flag. I'm talking about the other way, where the property takes care of keeping you up to date with it.

[sherm1]

I'm talking about the other way, where the property takes care of keeping you up to date with it.

That's not always as good as it sounds. Consider Matt Millard's curve Properties -- initialization requires an extremely expensive integration to generate the family of curves, so it is best to wait until all properties have been set. This could be dealt with by "lazy" initialization but that also has practical drawbacks including potentially large amounts of hidden computation, and error reports from those computations that don't appear to have anything to do with the call that initiated them. Generally I prefer to have a bullet-proof way to die if something hasn't been properly initialized, but make the initialization explicit.

[chrisdembia]

I will think about it and write you.

@mitkof6 That would be great.

That's not always as good as it sounds.

@sherm1 thank you for the elaboration.

[mitkof6]

Do you still think the Sandbox folder would be of interest to the new or typical user?

Ok. It is good to be mentioned in the main readme.