if we have radau and inverse dynamics constraints in collocation #448

Ipuch · 2022-03-02T20:59:00Z

All Submissions:

Have you followed the guidelines in our Contributing document [docs/contribution.md]?
Have you checked to ensure there aren't other open [Pull Requests] for the same update/change?
Have you opened/linked the issue related to your pull request?
Have you used the tag [WIP] for on-going changes, and removed it when the pull request was ready?
When ready to merge, have you sent a comment pinging @pariterre in it?

New Feature Submissions:

Does your submission pass the tests (if not please explain why this is intended)?
Did you write a proper documentation (docstrings and ReadMe)
Have you linted your code locally prior to submission (using the command: black . -l120 --exclude "external/*")?

Changes to Core Features:

Have you added an explanation of what your changes do and why you'd like us to include them?
Have you written new examples for your core changes, as applicable?
Have you written new tests for your core changes, as applicable?

according to
https://github.com/casadi/casadi/blob/402fe583f0d3cf1fc77d1e1ac933f75d86083124/casadi/solvers/collocation.cpp#L125

This change is

codecov · 2022-03-02T21:58:22Z

Codecov Report

Merging #448 (4969fb9) into CrazyDynamics (5d3d4ab) will increase coverage by 3.02%.
The diff coverage is 92.13%.

❗ Current head 4969fb9 differs from pull request most recent head fe6d5c6. Consider uploading reports for the commit fe6d5c6 to get more accurate results

@@                Coverage Diff                @@
##           CrazyDynamics     #448      +/-   ##
=================================================
+ Coverage          80.84%   83.86%   +3.02%     
=================================================
  Files                 85       86       +1     
  Lines               9299     9354      +55     
=================================================
+ Hits                7518     7845     +327     
+ Misses              1781     1509     -272

Impacted Files	Coverage Δ
bioptim/dynamics/integrator.py	`94.73% <66.66%> (-2.20%)`	⬇️
bioptim/dynamics/dynamics_functions.py	`95.72% <93.93%> (-0.47%)`	⬇️
bioptim/dynamics/configure_problem.py	`89.14% <95.45%> (+0.05%)`	⬆️
bioptim/__init__.py	`100.00% <100.00%> (ø)`
bioptim/dynamics/dynamics_evaluation.py	`100.00% <100.00%> (ø)`
bioptim/dynamics/ode_solver.py	`88.49% <100.00%> (+0.10%)`	⬆️
...ioptim/examples/getting_started/custom_dynamics.py	`88.88% <100.00%> (ø)`
...es/muscle_driven_ocp/muscle_activations_tracker.py	`75.23% <100.00%> (ø)`
...es/muscle_driven_ocp/muscle_excitations_tracker.py	`72.64% <100.00%> (ø)`
bioptim/examples/torque_driven_ocp/spring_load.py	`90.00% <100.00%> (ø)`
... and 4 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5d3d4ab...fe6d5c6. Read the comment docs.

Ipuch · 2022-03-03T13:46:53Z

related to #421

Ipuch · 2022-03-03T13:47:55Z

and #385

bioptim/dynamics/integrator.py

bioptim/examples/getting_started/pendulum.py

bioptim/dynamics/configure_problem.py

…ng defects for collocation

pariterre

Nice work so far!

bioptim/dynamics/configure_problem.py

bioptim/dynamics/dynamics_functions.py

bioptim/examples/getting_started/custom_dynamics.py

bioptim/examples/muscle_driven_ocp/muscle_activations_tracker.py

bioptim/examples/muscle_driven_ocp/muscle_excitations_tracker.py

tests/test_dynamics.py

tests/test_global_getting_started.py

Ipuch · 2022-03-10T00:03:26Z

I couldn't find out why the bimapping in test_symmetry_by_mapping(ode_solver) is not applied on qddot which is a states_dot. It fails in IRK and collocation then.

pariterre

Reviewed 3 of 11 files at r3, 12 of 12 files at r4, all commit messages.
Reviewable status: 15 of 16 files reviewed, 8 unresolved discussions (waiting on @Ipuch and @pariterre)

bioptim/dynamics/dynamics_functions.py, line 48 at r4 (raw file):

    @staticmethod
    def custom(states: MX.sym, controls: MX.sym, parameters: MX.sym, nlp):

The return should be -> DynamicsEvalution

bioptim/dynamics/dynamics_functions.py, line 102 at r4 (raw file):

        fatigue : FatigueList
            A list of fatigue elements
        """

Please add back the Returns, but adjust it to the DynamicsEvaluation class

bioptim/dynamics/dynamics_functions.py, line 116 at r4 (raw file):

            dxdt[nlp.states["qdot"].index, :] = DynamicsFunctions.get(nlp.controls["qddot"], controls)
        else:

Remove spacing

bioptim/dynamics/dynamics_functions.py, line 212 at r4 (raw file):

        with_contact: bool
            If the dynamic with contact should be used
        """

Please add back the Returns, but adjust it to the DynamicsEvaluation class

bioptim/dynamics/dynamics_functions.py, line 248 at r4 (raw file):

        with_contact: bool
            If the dynamic with contact should be used
        """

Please add back the Returns, but adjust it to the DynamicsEvaluation class

bioptim/dynamics/dynamics_functions.py, line 373 at r4 (raw file):

        with_torque: bool
            If the dynamic should be added with residual torques
        """

Please add back the Returns, but adjust it to the DynamicsEvaluation class

bioptim/examples/getting_started/custom_dynamics.py, line 76 at r4 (raw file):

    # the implicit dynamics f(x,u,p,xdot)=0 as the second argument
    # which may be useful for IRK or COLLOCATION integrators
    return DynamicsEvaluation(vertcat(dq, ddq), None)

Maybe add the hints here so lambda user sees what it means (DynamicsEvaluation(dxdt=..., ...=...))

bioptim/optimization/non_linear_program.py, line 119 at r4 (raw file):

        self.dt = None
        self.dynamics = []
        self.dynamics_sym = DynamicsEvaluation()

dynamics_sym? Why not dynamics_evalution as it is a DynamicsEvaluation class... Otherwise everyone has to remember that dynamics_sym is not an MX.sym instance, but a DynamicsEvaluation..

…_test

It looks fine like this

Ipuch · 2022-05-24T20:29:08Z

@pariterre reviewable, and I think mergeable on CrazyDyanmics.

Ipuch

Reviewable status: 2 of 17 files reviewed, 8 unresolved discussions (waiting on @pariterre)

bioptim/dynamics/dynamics_functions.py line 71 at r3 (raw file):

Previously, pariterre (Pariterre) wrote…

This should also take into account the dynamics when the user does not provide the implicit version.

I'm not sure to understand what you meant

bioptim/dynamics/dynamics_functions.py line 569 at r3 (raw file):

Previously, pariterre (Pariterre) wrote…

This is not tested but I feel it should be

this function was not used so deleted

bioptim/dynamics/dynamics_functions.py line 48 at r4 (raw file):

Previously, pariterre (Pariterre) wrote…

The return should be -> DynamicsEvalution

ok

bioptim/dynamics/dynamics_functions.py line 102 at r4 (raw file):

Previously, pariterre (Pariterre) wrote…

Please add back the Returns, but adjust it to the DynamicsEvaluation class

done

bioptim/dynamics/dynamics_functions.py line 116 at r4 (raw file):

Previously, pariterre (Pariterre) wrote…

Remove spacing

done

bioptim/dynamics/dynamics_functions.py line 212 at r4 (raw file):

Previously, pariterre (Pariterre) wrote…

Please add back the Returns, but adjust it to the DynamicsEvaluation class

done

bioptim/dynamics/dynamics_functions.py line 248 at r4 (raw file):

Previously, pariterre (Pariterre) wrote…

Please add back the Returns, but adjust it to the DynamicsEvaluation class

done

bioptim/dynamics/dynamics_functions.py line 373 at r4 (raw file):

Previously, pariterre (Pariterre) wrote…

Please add back the Returns, but adjust it to the DynamicsEvaluation class

done

bioptim/examples/getting_started/custom_dynamics.py line 76 at r4 (raw file):

Previously, pariterre (Pariterre) wrote…

Maybe add the hints here so lambda user sees what it means (DynamicsEvaluation(dxdt=..., ...=...))

done

bioptim/optimization/non_linear_program.py line 119 at r4 (raw file):

Previously, pariterre (Pariterre) wrote…

dynamics_sym? Why not dynamics_evalution as it is a DynamicsEvaluation class... Otherwise everyone has to remember that dynamics_sym is not an MX.sym instance, but a DynamicsEvaluation..

done

pariterre

Reviewed 1 of 11 files at r3, 4 of 4 files at r5, 10 of 12 files at r6, 2 of 2 files at r7, 1 of 1 files at r8, all commit messages.
Reviewable status: all files reviewed, 6 unresolved discussions (waiting on @Ipuch and @pariterre)

bioptim/dynamics/configure_problem.py line 160 at r7 (raw file):

        ConfigureProblem.configure_qdot(nlp, True, False, True)
        ConfigureProblem.configure_tau(nlp, False, True, fatigue)
        ConfigureProblem.configure_qddot(nlp, False, False, True)

move this to an else

bioptim/dynamics/configure_problem.py line 170 at r7 (raw file):

            or rigidbody_dynamics == RigidBodyDynamics.DAE_INVERSE_DYNAMICS_JERK
        ):
            ConfigureProblem.configure_qddot(nlp, True, False)

There is a bug here

bioptim/dynamics/integrator.py line 618 at r7 (raw file):

                xp_j += self._c[r, j] * states[r]

            if self.defects_type == "explicit":

DefectType.EXPLICIT
DefectType.IMPLICIT
DefectType.NOT_APPLICABLE

bioptim/dynamics/ode_solver.py line 237 at r7 (raw file):

        """

        def __init__(self, polynomial_degree: int = 4, method: str = "legendre", defects_type: str = "explicit"):

ENUM

bioptim/dynamics/ode_solver.py line 316 at r7 (raw file):

        """

        def __init__(self, polynomial_degree: int = 4, method: str = "legendre", defects_type: str = "explicit"):

ENUM

Ipuch

Reviewable status: all files reviewed, 6 unresolved discussions (waiting on @pariterre)

bioptim/dynamics/configure_problem.py line 160 at r7 (raw file):

Previously, pariterre (Pariterre) wrote…

move this to an else

not implemented yet

bioptim/dynamics/configure_problem.py line 170 at r7 (raw file):

Previously, pariterre (Pariterre) wrote…

There is a bug here

done

bioptim/dynamics/integrator.py line 618 at r7 (raw file):

Previously, pariterre (Pariterre) wrote…

DefectType.EXPLICIT
DefectType.IMPLICIT
DefectType.NOT_APPLICABLE

done

bioptim/dynamics/ode_solver.py line 237 at r7 (raw file):

Previously, pariterre (Pariterre) wrote…

ENUM

done

bioptim/dynamics/ode_solver.py line 316 at r7 (raw file):

Previously, pariterre (Pariterre) wrote…

ENUM

done

Ipuch · 2022-05-27T21:33:35Z

done.

pariterre

Reviewed 8 of 8 files at r9, 1 of 1 files at r10, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @Ipuch)

bioptim/examples/getting_started/pendulum.py line 34 at r10 (raw file):

    n_shooting: int,
    ode_solver: OdeSolver = OdeSolver.IRK(),
    use_sx: bool = False,

Please revert to the actual example

Code quote:

    ode_solver: OdeSolver = OdeSolver.IRK(),
    use_sx: bool = False,

pariterre

Reviewed 1 of 1 files at r11, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @Ipuch)

Ipuch added 2 commits March 2, 2022 15:54

if we have radau

018f752

Draft to handle inverse dynamics in collocations

e333acb

Ipuch changed the title ~~if we have radau~~ [WIP] if we have radau and inverse dynamics constraints in collocation Mar 2, 2022

pariterre requested changes Mar 3, 2022

View reviewed changes

Ipuch added 6 commits March 3, 2022 13:20

state_dot as a new optimizationVariable and implicit dynamics computi…

a99868a

…ng defects for collocation

associated files

a6d7918

Let's make the tests work

a3e165f

blacked

876134b

tricked the fatigue to make the tests work

b0e9260

updated readme

625b33b

pariterre requested changes Mar 8, 2022

View reviewed changes

Ipuch added 2 commits March 8, 2022 22:19

it partly fills the review

810f999

DynamicsEvaluation, an object to return dxdt and defects

b681586

pariterre requested changes Mar 11, 2022

View reviewed changes

Merge remote-tracking branch 'pyomeca/master' into collocation_test

e749a2c

Ipuch changed the base branch from master to CrazyDynamics April 29, 2022 20:19

Ipuch added 4 commits May 19, 2022 16:54

small adjustement on the docstring

36c700e

Merge remote-tracking branch 'pyomeca/CrazyDynamics' into collocation…

4685227

…_test

some review corrections

2b860c3

explicit or implicit defects

5af800b

It looks fine like this

blacked

9ab21fd

Ipuch changed the title ~~[WIP] if we have radau and inverse dynamics constraints in collocation~~ if we have radau and inverse dynamics constraints in collocation May 25, 2022

Moved the codecov test to MacOSx

4969fb9

Ipuch commented May 27, 2022

View reviewed changes

pariterre requested changes May 27, 2022

View reviewed changes

small hint on a function

6b808ab

Ipuch commented May 27, 2022

View reviewed changes

new enums and review

fe6d5c6

small fix on a constraint function

1991560

pariterre requested changes Jun 9, 2022

View reviewed changes

restored example

4201666

pariterre approved these changes Jun 9, 2022

View reviewed changes

pariterre merged commit 5be6662 into pyomeca:CrazyDynamics Jun 9, 2022

Ipuch deleted the collocation_test branch June 9, 2022 20:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

if we have radau and inverse dynamics constraints in collocation #448

if we have radau and inverse dynamics constraints in collocation #448

Ipuch commented Mar 2, 2022 •

edited

codecov bot commented Mar 2, 2022 •

edited

Ipuch commented Mar 3, 2022

Ipuch commented Mar 3, 2022

pariterre left a comment

Ipuch commented Mar 10, 2022

pariterre left a comment

Ipuch commented May 24, 2022

Ipuch left a comment

pariterre left a comment

Ipuch left a comment

Ipuch commented May 27, 2022

pariterre left a comment

pariterre left a comment

if we have radau and inverse dynamics constraints in collocation #448

if we have radau and inverse dynamics constraints in collocation #448

Conversation

Ipuch commented Mar 2, 2022 • edited

All Submissions:

New Feature Submissions:

Changes to Core Features:

codecov bot commented Mar 2, 2022 • edited

Codecov Report

Ipuch commented Mar 3, 2022

Ipuch commented Mar 3, 2022

pariterre left a comment

Choose a reason for hiding this comment

Ipuch commented Mar 10, 2022

pariterre left a comment

Choose a reason for hiding this comment

Ipuch commented May 24, 2022

Ipuch left a comment

Choose a reason for hiding this comment

pariterre left a comment

Choose a reason for hiding this comment

Ipuch left a comment

Choose a reason for hiding this comment

Ipuch commented May 27, 2022

pariterre left a comment

Choose a reason for hiding this comment

pariterre left a comment

Choose a reason for hiding this comment

Ipuch commented Mar 2, 2022 •

edited

codecov bot commented Mar 2, 2022 •

edited