[BREAKING] fix and update: holonomic constraint biais was wrong.#1044
[BREAKING] fix and update: holonomic constraint biais was wrong.#1044pariterre merged 27 commits intopyomeca:masterfrom
Conversation
|
@EveCharbie Could you please at least review the rigid contact defect consequences. Tell me if you have time to review the rest. Otherwise @pariterre. Please note there is a lot of doc and example clean up, and test values update, which explains the big amount of lines. Stay focus on the bioptim core first, instead of examples and tests. @p-shg consider reviewing the changes too, to prepare and update of your branch in #1042. (@fbailly ) |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #1044 +/- ##
==========================================
- Coverage 77.15% 76.94% -0.22%
==========================================
Files 193 192 -1
Lines 21019 21071 +52
==========================================
- Hits 16218 16213 -5
- Misses 4801 4858 +57
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Okay, don't you have any advice on the code smells that I could have done? |
|
Should we merge #1045 before merging here? |
pariterre
left a comment
There was a problem hiding this comment.
That is really satisfying to see all the clean up!
Thanks!
@pariterre reviewed 29 files and all commit messages, and made 3 comments.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @Ipuch).
bioptim/examples/toy_examples/holonomic_constraints/arm26_pendulum_swingup_muscle_algebraic.py line 210 at r1 (raw file):
if __name__ == "__main__": main() main()
two mains?
bioptim/examples/toy_examples/holonomic_constraints/arm26_pendulum_swingup_muscle.py line 42 at r1 (raw file):
from .custom_dynamics import HolonomicMusclesBiorbdModel except ImportError: from custom_dynamics import HolonomicMusclesBiorbdModel
This should not be mandatory... Have you tried adding a "init.py" file in the current folder?
or to move "custom_dynamics.py" in a "custom_dynamics" folder and put the code in "init.py" in that folder?
Ipuch
left a comment
There was a problem hiding this comment.
@Ipuch made 2 comments.
Reviewable status: 27 of 29 files reviewed, 2 unresolved discussions (waiting on @pariterre).
bioptim/examples/toy_examples/holonomic_constraints/arm26_pendulum_swingup_muscle.py line 42 at r1 (raw file):
Previously, pariterre (Pariterre) wrote…
This should not be mandatory... Have you tried adding a "init.py" file in the current folder?
or to move "custom_dynamics.py" in a "custom_dynamics" folder and put the code in "init.py" in that folder?
it didnot work
bioptim/examples/toy_examples/holonomic_constraints/arm26_pendulum_swingup_muscle_algebraic.py line 210 at r1 (raw file):
Previously, pariterre (Pariterre) wrote…
two mains?
done
|
@pariterre RTM for me. |
pariterre
left a comment
There was a problem hiding this comment.
@pariterre reviewed 12 files and all commit messages, and resolved 2 discussions.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on @Ipuch).
Pull Request Checklist
All Submissions:
New Feature Submissions:
black . -l120 --exclude "external/*")?Changes to Core Features:
Summary
Renames French "biais" to English "bias" throughout the holonomic constraints module. Also fixes a bug in rigid contact dynamics and cleans up examples.
Migration: Replace
biais→biasin your code:biais_vector()→bias_vector()_holonomic_constraints_biais→_holonomic_constraints_biascompute_biais_vector()→compute_bias_vector()Changes
1. API Rename (biais → bias)
_holonomic_constraints_biais_holonomic_constraints_biasholonomic_biorbd_model.pybiais_vector()bias_vector()holonomic_biomodel.pycompute_biais_vector()compute_bias_vector()holonomic_constraints.py2. API Change: Holonomic Constraint Return Values
The third return value of
HolonomicConstraintsFcnmethods is now a bias function (J̇q̇) instead of an acceleration constraint function. Users must compute the full acceleration constraint manually:Affected methods:
superimpose_markers(),align_frames(),rigid_contacts()3. Test Refactoring
test_biorbd_model_holonomic.py: Major cleanup (−1,310 / +1,027 lines)test_vector_layout.py: Updated bias reference4. Code Cleanup
common.pyReview Focus
biais→biasreplacementsImport Issue Fixes
Problem: Examples using
custom_dynamics.pyfailed when imported by tests withModuleNotFoundError: No module named 'custom_dynamics'Solution: Implemented try/except pattern for relative imports with fallback:
Files Fixed:
two_pendulums_algebraic.pyarm26_pendulum_swingup_muscle.pyarm26_pendulum_swingup_muscle_algebraic.pyTest Expectations Updated
Problem: Test failures due to numerical changes in
compute_q_from_u_iterativeimplementationSolution: Updated expected values in
test_biorbd_model_holonomic.pyfor:test_example_three_bartest_example_four_bartest_example_two_pendulums_2constrainttest_example_two_pendulums_rotuletest_example_arm26_pendulum_swingupAll tests now pass with the new implementation.
Code Quality Improvements
Fixed in 10+ example files:
import numpy as npstatementsFiles Cleaned:
two_pendulums.pytwo_pendulums_algebraic.pythree_bar.pyfour_bar.pytwo_pendulums_2constraint.pytwo_pendulums_2constraint_4DOF.pytwo_pendulums_rotule.pyarm26_pendulum_swingup.pyarm26_pendulum_swingup_muscle.pyarm26_pendulum_swingup_muscle_algebraic.pyDocumentation Enhancements
Module Documentation (
holonomic_constraints.py):HolonomicConstraintsFcnandHolonomicConstraintsListExample Docstrings:
This change is