[Wave RHO][1/4] Fix Acados control-bound scaling and ordering - #1075
[Wave RHO][1/4] Fix Acados control-bound scaling and ordering#1075mickaelbegon wants to merge 2 commits into
Conversation
2e09f75 to
9a88e00
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1075 +/- ##
==========================================
+ Coverage 77.26% 77.86% +0.60%
==========================================
Files 196 197 +1
Lines 21506 21504 -2
==========================================
+ Hits 16616 16744 +128
+ Misses 4890 4760 -130
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
RTM? looks good to me |
EveCharbie
left a comment
There was a problem hiding this comment.
And what about state and algebraic_state bounds ? Do they also need to be scaled in the same way ?
There was a problem hiding this comment.
This file addition is not necessary. Please instead use _dispatch_control_bounds from bound_vector.py. If not possible, please try to reuse existing code as much as possible and justify what is missing.
| self.acados_ocp.constraints.ubu = u_bounds_min | ||
| u_bounds_min, u_bounds_max = scaled_control_bounds(ocp.nlp[0]) | ||
| self.acados_ocp.constraints.lbu = u_bounds_min[:, np.newaxis] | ||
| self.acados_ocp.constraints.ubu = u_bounds_max[:, np.newaxis] |
There was a problem hiding this comment.
These self.acados_ocp.constraints.ubu should contain the scaled version. Please do not change the "meaning" of the attributes
There was a problem hiding this comment.
Could you please move these tests to test_bounds_accessor ? And please try to avoid mock classes like _Variables and use the real classes from bioptim as much as possible (if not possible please justify).
Note
Wave RHO 1/4 — Draft/WIP. Tracking issue: #1086.
This wave and its decomposition come from an analysis performed by OpenAI Codex. Maintainer review is required before these proposals are considered ready.
Wave RHO context
The overall goal is to upstream generic, application-independent foundations for reliable receding-horizon workflows with IPOPT and Acados: solver-input correctness, supported warm-start grid transfer, explicit window outcomes/diagnostics/hooks, and finally supported solver tuning.
Proposed review/merge order:
Role of this PR: correctness foundation for every Acados RHO run.
Interaction: Code-independent from #1073, but both establish the inputs used by the core RHO workflow in #1072.
All four PRs target
masterindependently; this is an interaction/review order, not a stacked Git dependency.What changed
lbuandubuin the correct lower/upper orderRoot cause
The generation path assigned maximum values to
lbuand minimum values toubu. The update path corrected the order but sent unscaled physical bounds to scaled Acados controls.Validation
pytest -q tests/shard1/test_acados_control_bounds.pyThis change is