opensim-org / opensim-core

SimTK OpenSim C++ libraries and command-line applications, and Java/Python wrapping.
https://opensim.stanford.edu
Apache License 2.0
796 stars 320 forks source link

CreateForceSetOfCoordinateActuatorsForModel causes segmentation fault through Python API #3598

Closed itbellix closed 12 months ago

itbellix commented 1 year ago

Hi, I am running into troubles when using the method CreateForceSetOfCoordinateActuatorsForModel for a CoordinateActuator object in the Python API.

The script that reproduces the error for me is the following:

model = opensim.Model(pathOpenSimModel)     # for example, the arm26.osim model
state = model.initSystem()

# append generalized forces to the model
coordActDummy = opensim.CoordinateActuator()    # dummy coordinate actuator to access required function
optimalForce = 1                                # optimal force [N or N/m] for every new actuator
includeLockedAndConstr = False                  # actuators are not created for locked and constrained coordinates
coordActDummy.CreateForceSetOfCoordinateActuatorsForModel(state, model, optimalForce, includeLockedAndConstr)

I have tested it with the arm26.osim model, but also with other models that I have, and the last line appears problematic (even when changing the includeLockedAndConstr to True). When running the the provided script, I have a segmentation fault error. However, this does not happen always (i.e., I was able to run the provided code once or twice, but not consistently).

I am using OpenSim in Ubuntu 20.04, through Python, and I see the same problem if I use a custom build of OpenSim, or if I use the conda package.

nickbianco commented 1 year ago

Hi @itbellix, thanks for the bug report. Not sure where the segfault is without setting up a build, but if you're looking for a way to add a set of CoordinateActuatorss to a model, you could use a ModelProcessor with ModOpAddReserves:

import opensim as osim

modelProcessor = osim.ModelProcessor(pathOpenSimModel)
optimalForce = 1
modelProcessor.append(osim.ModOpAddReserves(optimalForce))

model = modelProcessor.process()

This simply adds a set of CoordinateActuators to the model (for coordinates that do not already have an actuator associated with it), and doesn't mess around with ForceSet ownership or invalidating the system like CreateForceSetOfCoordinateActuatorsForModel does.

itbellix commented 1 year ago

Hi @nickbianco, thank you very much for your quick reply! Indeed, the code you provided has the behavior that I was trying to get, without resulting in errors :)

nickbianco commented 1 year ago

Great! We can hold on to this issue to decide what to do with CreateForceSetOfCoordinateActuatorsForModel. My preference would be to remove it, but we'd need to discuss if that would break a lot of legacy code.

nickbianco commented 12 months ago

It turns out CreateForceSetOfCoordinateActuatorsForModel is used by InverseDynamics and StaticOptimization, so we can't outright remove it without modifying those tools.

@itbellix you were probably getting a segfault because that function returns a raw pointer to a new ForceSet objects and the memory was not handled properly.