Fixing the violation of bounds when one of the bounds is inf in ActivationModelQuadraticBarrier
#1191
Conversation
There was a problem hiding this comment.
Thanks for fixing this issue, @akhilsathuluri!
I have left a few easy-to-do changes. However, I would also ask you to clean the history. This PR should have a single commit. Additionally, I would ask you to include one sentence description (inside the unreleased tag) in our CHANGELOG.md
Please ping me when you are done. Thanks again for your contribution!
| @@ -13,6 +13,7 @@ | |||
|
|
|||
| #include <pinocchio/utils/static-if.hpp> | |||
| #include <stdexcept> | |||
| #include <typeinfo> | |||
There was a problem hiding this comment.
We shouldn't need to include this. Could you remove it?
unittest/CMakeLists.txt
Outdated
| test_solvers | ||
| test_activation_bounds) |
There was a problem hiding this comment.
Let's move this new test to test_activations.cpp.
| test_solvers | |
| test_activation_bounds) | |
| test_solvers) |
unittest/test_activation_bounds.cpp
Outdated
| BOOST_CHECK(bounds.lb != m - beta * d); | ||
| } | ||
|
|
||
| bool register_test() { |
There was a problem hiding this comment.
When moving this into test_activations.cpp, please name this function as
| bool register_test() { | |
| bool register_bounds_unit_test() { |
There was a problem hiding this comment.
Hello @cmastalli, do you mean, move the new test into the existing test_activations.cpp file or create a new test named, test_activation.cpp? Further, in the previous comment you mention, test_activate.cpp? If a new test is to be created which of it would be the name?
There was a problem hiding this comment.
Yes, I meant test_activations.cpp. I apologize for the typos.
|
@akhilsathuluri fyi for the future, you can also force push the modifications to the same branch for which you have an open PR, instead of opening a new PR, if you want I can show you how to do next week. |
Changed in v2.1.0: * Updated black + isort + flake8 to ruff in loco-3d/crocoddyl#1256 * Exported version for Python in loco-3d/crocoddyl#1254 * Added pinocchio 3 preliminary support in loco-3d/crocoddyl#1253 * Updated CMake packaging in loco-3d/crocoddyl#1249 * Fixed ruff reported error in loco-3d/crocoddyl#1248 * Fixed yapf reported errors in loco-3d/crocoddyl#1238 * Tested Python stubs in Conda CI in loco-3d/crocoddyl#1228 * Fixed Rviz display in loco-3d/crocoddyl#1227 * Improved CI, updated cmake and fixed launch file in loco-3d/crocoddyl#1220 * Introduced a Rviz display in loco-3d/crocoddyl#1216 * Enabled display of thrust and simplied displayers code in loco-3d/crocoddyl#1215 * Introduced floating base thruster actuation model in loco-3d/crocoddyl#1213 * Fixed quadruped and biped examples in loco-3d/crocoddyl#1208 * Fixed terminal computation in Python models in loco-3d/crocoddyl#1204 * Fixed handling of unbounded values for `ActivationBounds` in loco-3d/crocoddyl#1191
Changed in v2.1.0: * Updated black + isort + flake8 to ruff in loco-3d/crocoddyl#1256 * Exported version for Python in loco-3d/crocoddyl#1254 * Added pinocchio 3 preliminary support in loco-3d/crocoddyl#1253 * Updated CMake packaging in loco-3d/crocoddyl#1249 * Fixed ruff reported error in loco-3d/crocoddyl#1248 * Fixed yapf reported errors in loco-3d/crocoddyl#1238 * Tested Python stubs in Conda CI in loco-3d/crocoddyl#1228 * Fixed Rviz display in loco-3d/crocoddyl#1227 * Improved CI, updated cmake and fixed launch file in loco-3d/crocoddyl#1220 * Introduced a Rviz display in loco-3d/crocoddyl#1216 * Enabled display of thrust and simplied displayers code in loco-3d/crocoddyl#1215 * Introduced floating base thruster actuation model in loco-3d/crocoddyl#1213 * Fixed quadruped and biped examples in loco-3d/crocoddyl#1208 * Fixed terminal computation in Python models in loco-3d/crocoddyl#1204 * Fixed handling of unbounded values for `ActivationBounds` in loco-3d/crocoddyl#1191
Changed in v2.1.0: * Updated black + isort + flake8 to ruff in loco-3d/crocoddyl#1256 * Exported version for Python in loco-3d/crocoddyl#1254 * Added pinocchio 3 preliminary support in loco-3d/crocoddyl#1253 * Updated CMake packaging in loco-3d/crocoddyl#1249 * Fixed ruff reported error in loco-3d/crocoddyl#1248 * Fixed yapf reported errors in loco-3d/crocoddyl#1238 * Tested Python stubs in Conda CI in loco-3d/crocoddyl#1228 * Fixed Rviz display in loco-3d/crocoddyl#1227 * Improved CI, updated cmake and fixed launch file in loco-3d/crocoddyl#1220 * Introduced a Rviz display in loco-3d/crocoddyl#1216 * Enabled display of thrust and simplied displayers code in loco-3d/crocoddyl#1215 * Introduced floating base thruster actuation model in loco-3d/crocoddyl#1213 * Fixed quadruped and biped examples in loco-3d/crocoddyl#1208 * Fixed terminal computation in Python models in loco-3d/crocoddyl#1204 * Fixed handling of unbounded values for `ActivationBounds` in loco-3d/crocoddyl#1191
This PR fixes the issue mentioned in #1129.
Now when one of the
ubor thelbarenanorinf, once they are converted tostd::numeric_limits<Scalar>::max()/min(), such bounds are no longer modified even when a valid value forbetais specified.Thanks to @traversaro, an associated test function in
test_activation_bounds.cppis added, which fails when the bounds are not respected.