opensim-org / opensim-core

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

Upgrade casadi/ipopt stack #3693

Closed aymanhab closed 1 month ago

aymanhab commented 5 months ago

Fixes issue #0 Initially the plan was to upgrade to a more stable/tagged ipopt (as the branch name suggests), however it appears latest casadi release is built against custom ipopt repo/version based on 3.14.11.mod because of that I'm leaning to remove the separate ipopt dependency and instead have casadi download/install its own dependencies.

Brief summary of changes

Upgrade to more recent casadi/ipopt stack

Testing I've completed

Looking for feedback on...

CHANGELOG.md (choose one)


This change is Reviewable

aymanhab commented 5 months ago

@nickbianco Would like to hear your thoughts on the description of what this PR wants to do and if you foresee problems with this approach.

nickbianco commented 5 months ago

@aymanhab, that sounds reasonable to me, assuming that the version of Ipopt plays nice with all non-CasADi Ipopt usage in OpenSim. Is this version of Ipopt compatible with Mac Silicon?

aymanhab commented 5 months ago

Considering that casadi uses a custom ipopt internally, using the latest tagged/public version seems more problematic than needed, so will stick with the version maintained by Joris at https://github.com/jgillis/Ipopt-1.git to make sure latest casadi works then test if tropter can use it.

aymanhab commented 4 months ago

Building latest casadi with its own ipopt on linux succeeds but fails one moco related test after fixing 2 compilation errors. Refactoring to use the same ipopt for both casdai and tropter before asking for review. Windows has a totally different pathway in CMake that fails to build out-of-the-box. Investigating

aymanhab commented 4 months ago

@carmichaelong I have a conda package on linux (opensim-moco 4.5.1 opensim_admin) including the updated casadi (no tropter) here, any idea how to test this? Thank you https://anaconda.org/opensim_admin/opensim-moco/files

nickbianco commented 4 months ago

@aymanhab @carmichaelong MocoCasADiSolver is the default solver for pretty much all the Moco examples, if you wanted to run those as a starting point.

aymanhab commented 4 months ago

@nickbianco or @carmichaelong Would it be possible to take a look at the changes to the two header files in this PR and the Moco test failures to let me know if these are signs of bigger problems? Also I made a conda package on linux that I would like to test on jupyter. If you have or can point to a short snippet/example that exercises casadi I'd appreciate it.

nickbianco commented 4 months ago

@aymanhab I'm not too worried about the test failures on mac and linux. I think the moco track test is using looser tolerances to keep runtime down, and test moco implicit was already unstable before this pr. I'd like to take a closer look at both tests to confirm, but I don't see any signs of bigger problems yet.

aymanhab commented 4 months ago

Installing from Anaconda.org and running the slidingMassExample.py I get the following

[info] ======================================================================== [info] MocoCasADiSolver starting. [info] Mon Mar 4 22:15:10 2024 [info] ------------------------------------------------------------------------ Costs: (total: 1) goal. MocoFinalTimeGoal, enabled: true, mode: cost, weight: 1.0 Endpoint constraints: none Kinematic constraints: none Path constraints: none States: (total: 2) /slider/position/speed. bounds: [-50, 50] initial: 0 final: 0 /slider/position/value. bounds: [-5, 5] initial: 0 final: 1 Controls: (total: 1) /actuator. bounds: [-50, 50] Parameters: none [info] Number of threads: 8 CasADi - 2024-03-04 22:15:10 WARNING("CasADi was not compiled with WITH_THREAD=ON. Falling back to serial evaluation.") [.../casadi/core/map.cpp:507] CasADi - 2024-03-04 22:15:10 WARNING("CasADi was not compiled with WITH_THREAD=ON. Falling back to serial evaluation.") [.../casadi/core/map.cpp:507] CasADi - 2024-03-04 22:15:10 WARNING("CasADi was not compiled with WITH_THREAD=ON. Falling back to serial evaluation.") [.../casadi/core/map.cpp:507] CasADi - 2024-03-04 22:15:10 WARNING("CasADi was not compiled with WITH_THREAD=ON. Falling back to serial evaluation.") [.../casadi/core/map.cpp:507] CasADi - 2024-03-04 22:15:10 WARNING("CasADi was not compiled with WITH_THREAD=ON. Falling back to serial evaluation.") [.../casadi/core/map.cpp:507] CasADi - 2024-03-04 22:15:10 WARNING("CasADi was not compiled with WITH_THREAD=ON. Falling back to serial evaluation.") [.../casadi/core/map.cpp:507]

List of user-set options:

                                Name   Value                used
               hessian_approximation = limited-memory        yes
                       linear_solver = mumps                 yes
                  print_user_options = yes                   yes

This program contains Ipopt, a library for large-scale nonlinear optimization. Ipopt is released as open source code under the Eclipse Public License (EPL). For more information visit https://github.com/coin-or/Ipopt


This is Ipopt version 3.14.14, running with linear solver MUMPS 5.6.2.

Number of nonzeros in equality constraint Jacobian...: 3284 Number of nonzeros in inequality constraint Jacobian.: 0 Number of nonzeros in Lagrangian Hessian.............: 0

Total number of variables............................: 600 variables with only lower bounds: 0 variables with lower and upper bounds: 600 variables with only upper bounds: 0 Total number of equality constraints.................: 500 Total number of inequality constraints...............: 0 inequality constraints with only lower bounds: 0 inequality constraints with lower and upper bounds: 0 inequality constraints with only upper bounds: 0

iter objective inf_pr inf_du lg(mu) ||d|| lg(rg) alpha_du alpha_pr ls 0 2.5000000e+00 1.00e+00 1.00e+00 0.0 0.00e+00 - 0.00e+00 0.00e+00 0 1 1.9444444e+00 6.04e-03 2.23e+02 0.0 2.30e+00 - 9.65e-01 1.00e+00h 1 2 2.1611145e+00 8.90e-04 1.70e+01 -0.8 8.66e-01 - 9.97e-01 1.00e+00h 1 3 2.1407792e+00 7.11e-06 1.94e+00 -2.5 7.36e-02 - 1.00e+00 1.00e+00h 1 4 2.0136752e+00 2.26e-04 5.19e-01 -3.9 3.74e-01 - 1.00e+00 1.00e+00f 1 5 6.2200799e-01 3.11e-02 2.34e-01 -4.9 4.70e+00 - 1.00e+00 1.00e+00f 1 6 1.2683994e+00 1.25e-03 3.29e-01 -4.1 6.46e-01 - 1.00e+00 1.00e+00h 1 7 1.2578103e+00 3.66e-05 6.25e-02 -5.7 7.28e-01 - 1.00e+00 1.00e+00h 1 8 1.0638471e+00 2.35e-03 8.48e-01 -6.8 2.54e+00 - 1.00e+00 1.00e+00f 1 9 1.0638461e-02 1.23e-01 2.41e-01 -6.0 5.98e+01 - 1.00e+00 4.09e-01f 1 iter objective inf_pr inf_du lg(mu) ||d|| lg(rg) alpha_du alpha_pr ls 10 4.2212625e-01 1.28e-02 1.16e-02 -4.0 2.46e+01 - 1.00e+00 7.67e-01h 1 11 5.7667687e-01 8.21e-03 7.73e-02 -4.3 1.07e+01 - 7.32e-01 1.00e+00h 1 12 5.4233562e-01 1.14e-03 1.24e-02 -5.2 7.18e+00 - 1.00e+00 1.00e+00h 1 13 5.2643649e-01 1.30e-03 2.12e-02 -4.6 3.63e+01 - 1.00e+00 9.85e-02h 1 14 4.9264115e-01 2.04e-03 1.30e-02 -4.4 4.02e+01 - 1.00e+00 1.98e-01h 1 15 4.5221698e-01 2.82e-03 1.47e-02 -3.8 2.83e+01 - 1.00e+00 4.35e-01h 1 16 4.2721357e-01 2.23e-03 3.52e-02 -3.8 1.86e+01 - 1.00e+00 7.63e-01h 1 17 4.1549867e-01 1.23e-03 1.62e-02 -4.0 2.08e+01 - 1.00e+00 9.20e-01h 1 18 4.0648226e-01 1.00e-03 9.47e-03 -4.5 2.25e+01 - 1.00e+00 9.43e-01h 1 19 4.0208321e-01 4.38e-04 2.53e-03 -5.0 2.05e+01 - 1.00e+00 9.87e-01h 1 iter objective inf_pr inf_du lg(mu) ||d|| lg(rg) alpha_du alpha_pr ls 20 4.0106219e-01 9.45e-05 1.25e-03 -5.3 1.94e+01 - 1.00e+00 9.65e-01h 1 21 4.0054221e-01 3.94e-05 4.16e-04 -5.8 1.50e+01 - 1.00e+00 7.21e-01h 1 22 4.0003142e-01 2.20e-06 1.82e-03 -7.8 8.61e-01 - 1.00e+00 1.00e+00h 1 23 4.0002688e-01 2.93e-09 6.80e-10 -8.9 5.96e-02 - 1.00e+00 9.99e-01h 1

Number of Iterations....: 23

                               (scaled)                 (unscaled)

Objective...............: 4.0002687596284742e-01 4.0002687596284742e-01 Dual infeasibility......: 6.8049239666605364e-10 6.8049239666605364e-10 Constraint violation....: 2.9290379011115419e-09 2.9290379011115419e-09 Variable bound violation: 0.0000000000000000e+00 0.0000000000000000e+00 Complementarity.........: 2.0557576002094409e-09 2.0557576002094409e-09 Overall NLP error.......: 2.9290379011115419e-09 2.9290379011115419e-09

Number of objective function evaluations = 24 Number of objective gradient evaluations = 24 Number of equality constraint evaluations = 24 Number of inequality constraint evaluations = 0 Number of equality constraint Jacobian evaluations = 24 Number of inequality constraint Jacobian evaluations = 0 Number of Lagrangian Hessian evaluations = 0 Total seconds in IPOPT = 1.163

EXIT: Optimal Solution Found. nlp : t_proc (avg) t_wall (avg) n_eval callback_fun | 962.00us ( 40.08us) 966.28us ( 40.26us) 24 nlp_f | 275.00us ( 11.46us) 275.66us ( 11.49us) 24 nlp_g | 27.87ms ( 1.16ms) 27.85ms ( 1.16ms) 24 nlp_grad_f | 1.95ms ( 75.04us) 1.95ms ( 75.14us) 26 nlp_jac_g | 1.05 s ( 42.10ms) 1.05 s ( 42.10ms) 25 total | 1.16 s ( 1.16 s) 1.16 s ( 1.16 s) 1

Breakdown of objective (including weights): goal: 0.400027 [info] Set log level to Info. [info] ------------------------------------------------------------------------ [info] Elapsed real time: 1 second(s). [info] Mon Mar 4 22:15:11 2024 [info] MocoCasADiSolver succeeded! [info] ======================================================================== Traceback (most recent call last): File "/home/ayman/Downloads/OpenSim-4.5-2024-02-26-03d3ad7-ub20-linux/OpenSim-4.5-2024-02-26-03d3ad7/opensim/Resources/Code/Python/Moco/exampleSlidingMass.py", line 92, in study.visualize(solution); File "/home/ayman/miniconda3/envs/freshpy39/lib/python3.9/site-packages/opensim/moco.py", line 5000, in visualize return _moco.MocoStudy_visualize(self, it) RuntimeError: std::exception in 'void OpenSim::MocoStudy::visualize(OpenSim::MocoTrajectory const &) const': SimTK Exception thrown at VisualizerProtocol.cpp:167: Error detected by Simbody method VisualizerProtocol::ctor(): Unable to spawn executable 'simbody-visualizer' from directories: /home/ayman/miniconda3/envs/freshpy39/bin/ /home/ayman/miniconda3/envs/freshpy39/bin/ /home/ayman/miniconda3/condabin/ /usr/local/sbin/ /usr/local/bin/ /usr/sbin/ /usr/bin/ /sbin/ /bin/ /usr/games/ /usr/local/games/ /snap/bin/ /home/ayman/miniconda3/envs/freshpy39/libexec/simbody/ /home/ayman/miniconda3/envs/freshpy39/simbody/libexec/simbody/Final system error was errno=2 (No such file or directory). (Required condition 'status == 0' was not met.)

Traceback (most recent call last): File "/home/ayman/Downloads/OpenSim-4.5-2024-02-26-03d3ad7-ub20-linux/OpenSim-4.5-2024-02-26-03d3ad7/opensim/Resources/Code/Python/Moco/exampleSlidingMass.py", line 92, in study.visualize(solution); File "/home/ayman/miniconda3/envs/freshpy39/lib/python3.9/site-packages/opensim/moco.py", line 5000, in visualize return _moco.MocoStudy_visualize(self, it) RuntimeError: std::exception in 'void OpenSim::MocoStudy::visualize(OpenSim::MocoTrajectory const &) const': std::exception

The visualizer issue is no surprise but no issues with locating or tunning Casadi/ipopt. Same should be true on jupyter using conda install opensim_admin::opensim-moco

For your review/assessment @nickbianco or @carmichaelong

nickbianco commented 4 months ago

Mostly looks fine, but based on this error it doesn't seem like multithreading is available:

CasADi - 2024-03-04 22:15:10 WARNING("CasADi was not compiled with WITH_THREAD=ON. Falling back to serial evaluation.") [.../casadi/core/map.cpp:507]

aymanhab commented 4 months ago

Ok, fixed now and all threads are used and can use in Jupyter notebook online as well. Thank you @nickbianco

nickbianco commented 4 months ago

@aymanhab, since you're already working on the Ipopt/CasADi build stack, can I request to add the fix for #3265 to this PR? Based on the CasADi GitHub wiki, this should only involve copying some header files.

Let me know what you I think.

aymanhab commented 2 months ago

@nickbianco This PR upgrades ipopt to 3.14.16 and builds casadi and tropter solver on linux, mac and casadi only on windows. If we can test performance differences and assess the test failures to see if we should update the standard or investigate further that would be awesome. Will not merge until tropter on windows issue is resolved so there's no loss of functionality. Thank you

nickbianco commented 1 month ago

@aymanhab, I've looked over the failures and they seem mostly minor to me.

test_generic_optimization.cpp

/Users/runner/work/opensim-core/opensim-core/Vendors/tropter/tests/test_generic_optimization.cpp:143: FAILED:
  REQUIRE( solution.variables[0] == 1.0 )
with expansion:
  0.99999999 == 1.0

Obviously the solution here is very close to the known solution, so I think minor differences between the Ipopt version and the dependencies are leading to slightly different results. Looking at the test file, the REQUIRE check for this failure the only one not using Approx on the solution variable, which allows for approximate equality. I think we could just add Approx here (and the other similar failing checks in this file) and move on.

testMocoImplicit.cpp

/Users/runner/work/opensim-core/opensim-core/OpenSim/Moco/Test/testMocoImplicit.cpp:166: FAILED:
  CHECK( solutionImplicit.getFinalTime() == Approx(solution.getFinalTime()).margin(1e-2) )
with expansion:
  0.5742825471 == Approx( 0.623126902 )
with messages:
  solutionImplicit.getFinalTime() := 0.5742825471
  solution.getFinalTime() := 0.623126902
  stateError := 1.6862908536
  controlError := 35.3049664198

/Users/runner/work/opensim-core/opensim-core/OpenSim/Moco/Test/testMocoImplicit.cpp:168: FAILED:
  CHECK( controlError < 30.0 )
with expansion:
  35.3049664198 < 30.0
with messages:
  solutionImplicit.getFinalTime() := 0.5742825471
  solution.getFinalTime() := 0.623126902
  stateError := 1.6862908536
  controlError := 35.3049664198

This failure in testMocoImplicit.cpp looks worse, but these tests that compare implict dynamics versus explicit dynamics have always been flakey. Notice that the test tolerances are very large here. This test might just be poorly designed, and I'm somewhat inclined to remove it or at least comment it out until we improve it.

testMocoInterface.cpp

/Users/runner/work/opensim-core/opensim-core/OpenSim/Moco/Test/testMocoInterface.cpp:1293: FAILED:
  REQUIRE( solutionSim.compareContinuousVariablesRMS(guess) < 1e-2 )
with expansion:
  0.1262155023 < 0.01

This one I'm not sure about. The error is larger than Approx would resolve, but the test tolerance was already pretty wide to begin with.

python_tests

FAIL: test_changing_time_bounds (test_moco.TestWorkflow)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/runner/work/opensim-core/opensim-core/Bindings/Python/tests/test_moco.py", line 335, in test_changing_time_bounds
    self.assertAlmostEqual(solution.getFinalTime(), 5.8)
AssertionError: 5.799999942009047 != 5.8 within 7 places (5.7990952484487934e-08 difference)

This one seems similar to the test_generic_optimization.cpp failure. We could use a slightly more generous tolerance and move on.

nickbianco commented 1 month ago

I looked at the testMocoInterface.cpp test again. That test is comparing a Moco simulation using no costs and zero applied torque to a guess created with time-stepping integration (also no applied torque). So I would expect the trajectories between the simulation and time-stepping solution to be more similar. It would be worth plotting the two solutions in this test to see what's going on.

aymanhab commented 1 month ago

Thanks @nickbianco Will definitely look more into that specific failure and modify the tolerances on the rest as you suggested πŸ‘

aymanhab commented 1 month ago

With the latest commits, the following failures require more investigation maybe with help from @nickbianco casadiSolver: testMocoInterface, testMocoImplicit (Mac), testMocoTrack (linux) run log here https://github.com/opensim-org/opensim-core/actions/runs/9087361946 With tropter these additional tests fail tropter: testMocoInterface (Win), testMocoInterface (Mac) test_double_pendulum, testMocoInterface (linux) run log here https://github.com/opensim-org/opensim-core/actions/runs/9074873113

nickbianco commented 1 month ago

@aymanhab I ran the testMocoInterface time-stepping test on a local build. Weirdly, the tests pass for me but the comparison between the timestepping guess and the simulation solution look the same as what you looked at in your office:

image

Maybe the simulation solution differed from the timestepping guess more than I am remembering. The error between the two trajectories are calculated the same, and the timestepping guess shouldn't change with the Ipopt update, so the trajectory from the simulation must be different. But it's strange that only this test would produce such a difference.

nickbianco commented 1 month ago

I've ran the testMocoInterface test on both main and the current working branch, and it turns out the the problem is not solving correctly after all. As you can see, the problem is not strictly enforcing the bounds on the actuator (it should be exactly zero). The bounds on the initial state are also be very slightly violated.

The test fails with both the CasADi and tropter solver. Not really sure why this is happening. :shrug:

testMocoInterface_guess_timestepping

nickbianco commented 1 month ago

I found the issue! When running the problem in CasADi, I got a warning that the problem was overconstrained. Why Ipopt doesn't complain and why it solved successfully before, I'm not sure. But if you remove the coordinate actuator from the pendulum model in the test (since it is constrained to zero anyway), then the test passes.

I'll create a PR to patch into this branch.

nickbianco commented 1 month ago

Merged in the fix for the testMocoInterface test. I'll look into fixes for the other test failures.

aymanhab commented 1 month ago

Awesome, thanks @nickbianco

aymanhab commented 1 month ago

All tests now pass on all platforms thanks much @nickbianco for the fixes and the review.

nickbianco commented 1 month ago

@aymanhab My bad: the Mac test failed again because the tagging logic only applies to tests defined using OpenSimAddTests. If you add that logic to MocoAddTest here it should work as expected.

nickbianco commented 1 month ago

We should eventually have Moco tests use OpenSimAddTests instead of the Moco-specific macro, but we can handle that in a future PR.

aymanhab commented 1 month ago

No worries, will do. Thanks @nickbianco

aymanhab commented 1 month ago

Ready for review @nickbianco

aymanhab commented 1 month ago

Awesome thanks much for all your help and review @nickbianco πŸ‘