opensim-org / opensim-core

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

Maintain support for workflow using `PrescribedController::prescribeControlForActuator()` with model and controller loaded from file #3749

Closed carmichaelong closed 1 month ago

carmichaelong commented 3 months ago

A workflow for PrescribedController is to load a model with a controller and update a Function using prescribeControlForActuator(). Updating the PrescribedContoller class to use sockets has changed how this use case works, as prescribeControlForActuator() appends the function to the end of the ControlFunctions property, which can cause a couple issues (files attached that can be used to replicate these):

  1. Appending an additional function means that the number of control functions is not equal to the number of socket connectees, throwing here.
  2. In the case where the Function name is already the actuator name, using prescribeControlForActuator() with the actuator name will still append another Function (and now ControlFunctions will have 2 Functionss of the same name)

PrescribedControllerExample.zip using 4.5-2024-03-11-765d625

A couple of ideas chatted about:

  1. Relax the check (to allow for more control functions than actuators.
  2. prescribeControlForActuator() could try to use the map to see if an actuator exists and update the associated Function (I'm a little surprised this doesn't happen here, perhaps there's a mix between just using the actuator name vs full path in the map?)
carmichaelong commented 3 months ago

cc @nickbianco

nickbianco commented 3 months ago

(I'm a little surprised this doesn't happen here, perhaps there's a mix between just using the actuator name vs full path in the map?)

Yes, I think that is the issue. A user could have the label /forceset/soleus_r in the map, but if they provide the label soleus_r with a new Function it will not overwrite the function in ControlFunctions. We should also clarify in the docs, that while Sockets generally allow specifying components with the same name as long as they have different paths (i.e., /forceset/actuator vs /actuator), PrescribedController will not since we're still supporting legacy naming behavior here.