opensim-org / opensim-core

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

"max_control" and "min_control" Actuator properties are generally not respected #2035

Open carmichaelong opened 6 years ago

carmichaelong commented 6 years ago

Searching for getMaxControl and getMinControl shows that these are only used in SO and CMC. This is very confusing in any other case since it seems like setting these properties are ignored without any warning or error. Possible solutions: 1) Allow users to set the control to be outside of the min and max bounds. Clamp control (i.e., at setControls) and document the clamping. 2) Allow users to set the control to be outside of the min and max bounds. Throw a warning that the control was set outside of the bounds and clamp. 3) Throw an exception if a control is set outside of the bounds.

chrisdembia commented 6 years ago

It would be good to consider how this decision affects direct collocation, where we perturb a control value (perhaps out of bounds) to compute a finite difference. Clamping would cause the finite difference to report a derivative of 0; maybe that's okay but I'm not sure. If necessary, we could have "strict" and "lenient" modes.

carmichaelong commented 6 years ago

where we perturb a control value (perhaps out of bounds) to compute a finite difference

Perhaps in this type of formulation, the direct collocation method has knowledge of the bounds (and should in the end only report a feasible solution) but the model should have no bounds on its controls (i.e., the modeler should set max_control and min_control to inf and -inf)?

chrisdembia commented 6 years ago

I was hoping to use the user-specified max_control etc. to set the bounds on the variable in the direct collocation problem. But, after I access the value, I could alter the max_control in the model to be inf. Good idea.

aseth1 commented 6 years ago

I like option 3. the best. If the user is setting the max/min, we should throw when ever that is being violated. Solvers, like direct collocation, must in the end respect the bounds, but if they need to relax the bounds to compute derivatives, etc., they should be able to do that. This could take different forms, either removing the control limits, or setting wider bounds.

My least favorite is clamping, where neither the user nor the solver will be aware of what is really going on.

carmichaelong commented 6 years ago

If the user is setting the max/min, we should throw when ever that is being violated.

I think this is generally a good point and should definitely be implemented in the current methods for setting controls.

Additionally to this, would it be useful to implement a new method which allows you to set controls without checking bounds? Could this be useful for the case @chrisdembia described above since if you know for sure what you're doing, you can skip some if statement checks?

chrisdembia commented 6 years ago

Additionally to this, would it be useful to implement a new method which allows you to set controls without checking bounds? Could this be useful for the case @chrisdembia described above since if you know for sure what you're doing, you can skip some if statement checks?

Ah interesting. That could be nice.

carmichaelong commented 5 years ago

@chrisdembia @aseth1 I'm pinging this issue again as it has popped up in the osim-rl issue linked above. Although I don't think it's critical for 4.0, I think it should be considered soon after.