-
Notifications
You must be signed in to change notification settings - Fork 46
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[RTR]PR changes for ligaments #614
Conversation
few changes to prepare arrival of ligaments in bioptim
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## master #614 +/- ##
==========================================
- Coverage 81.69% 81.67% -0.03%
==========================================
Files 107 107
Lines 11849 11850 +1
==========================================
- Hits 9680 9678 -2
- Misses 2169 2172 +3
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 6 of 6 files at r1, 3 of 3 files at r2, 1 of 3 files at r3, 9 of 9 files at r4, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @AntoineLeroy)
tests/test_ligaments.py
line 357 at r4 (raw file):
# initial and final position np.testing.assert_almost_equal(q[:, 0], np.array([0.0])) np.testing.assert_almost_equal(q[:, -1], np.array([-3.1415927]))
Why these values changed?
Code quote:
3.1415927])
bioptim/examples/torque_driven_ocp/ocp_mass_with_ligament.py
line 72 at r4 (raw file):
) u_init.add([tau_init] * bio_model.nb_tau) else:
please use elif for all of the case and raise a NotImplementedYet if it gets to else
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 10 of 11 files reviewed, 2 unresolved discussions (waiting on @pariterre)
tests/test_ligaments.py
line 357 at r4 (raw file):
Previously, pariterre (Pariterre) wrote…
Why these values changed?
because these are the right values for the test to pass
bioptim/examples/torque_driven_ocp/ocp_mass_with_ligament.py
line 72 at r4 (raw file):
Previously, pariterre (Pariterre) wrote…
please use elif for all of the case and raise a NotImplementedYet if it gets to else
Done.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 1 of 1 files at r5, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @AntoineLeroy)
tests/test_ligaments.py
line 357 at r4 (raw file):
Previously, AntoineLeroy wrote…
because these are the right values for the test to pass
I understand that these are the current values for the tests to pass, but why did it changed? Making the tests passing is not the goal, the goal is that it returns the right answer. I am asking, what did you change that made these values change and are we sure it is intended
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @pariterre)
tests/test_ligaments.py
line 357 at r4 (raw file):
Previously, pariterre (Pariterre) wrote…
I understand that these are the current values for the tests to pass, but why did it changed? Making the tests passing is not the goal, the goal is that it returns the right answer. I am asking, what did you change that made these values change and are we sure it is intended
This is a test I created specifically for this PR and the change is from RigidBodyDynamics.ODE to RigidBodyDynamics.DAE_FORWARD_DYNAMICS and RigidBodyDynamics.DAE_FINVERSE_DYNAMICS. I just wanted to test the last two.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @AntoineLeroy)
few changes to prepare arrival of ligaments in bioptim
All Submissions:
New Feature Submissions:
black . -l120 --exclude "external/*"
)?Changes to Core Features:
This change is