Skip to content
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

feat: pyorerun interfaced #882

Merged
merged 21 commits into from
Jul 19, 2024
Merged

feat: pyorerun interfaced #882

merged 21 commits into from
Jul 19, 2024

Conversation

Ipuch
Copy link
Collaborator

@Ipuch Ipuch commented Jun 24, 2024

This change is Reviewable

@Ipuch Ipuch changed the title feat: pyorerun interfaced [RTR] feat: pyorerun interfaced Jul 4, 2024
@Ipuch Ipuch requested a review from EveCharbie July 4, 2024 14:24
@Ipuch
Copy link
Collaborator Author

Ipuch commented Jul 4, 2024

@EveCharbie if you like to review, you can do it, otherwise it can wait for @pariterre returns.

Copy link
Collaborator

@EveCharbie EveCharbie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you handle quaternions? If not, please raise an error.

Reviewed 17 of 27 files at r1, 1 of 1 files at r2, all commit messages.
Reviewable status: 18 of 28 files reviewed, 5 unresolved discussions (waiting on @Ipuch)


bioptim/examples/getting_started/pendulum.py line 131 at r2 (raw file):

    # --- Prepare the ocp --- #
    ocp = prepare_ocp(biorbd_model_path="models/pendulum.bioMod", final_time=1, n_shooting=30, n_threads=2)

revert?

Code quote:

30

bioptim/examples/getting_started/pendulum.py line 160 at r2 (raw file):

    # --- Animate the solution --- #
    viewer = "bioviz"

make an enum

Code quote:

"bioviz"

bioptim/examples/holonomic_constraints/two_pendulums.py line 229 at r2 (raw file):

        viz.add_animated_model(pyorerun.BiorbdModel(model_path), q=q)

        viz.rerun("double_pendulum")

Super clean!


bioptim/models/biorbd/biorbd_model.py line 810 at r2 (raw file):

            raise RuntimeError("pyorerun must be install to animate the model")

        # check_version(pyorerun, "1.2.3", max_version="1.2.99")

Do we need this? If yes, uncomment please

Code quote:

check_version(pyorerun, "1.2.3", max_version="1.2.99")

bioptim/models/biorbd/viewer_bioviz.py line 116 at r2 (raw file):

            else:
                b_is_visible[i] = False
    return None

Was it like that? I think viz.exec() does this

Code quote:

def play_bioviz_animation(all_bioviz: list) -> None:
    """Play the animation of the list of bioviz objects"""
    b_is_visible = [True] * len(all_bioviz)
    while sum(b_is_visible):
        for i, b in enumerate(all_bioviz):
            if b.vtk_window.is_active:
                b.update()
            else:
                b_is_visible[i] = False
    return None

@EveCharbie
Copy link
Collaborator

Thanks @Ipuch, this will be very helpful and clean!

Copy link
Collaborator Author

@Ipuch Ipuch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Quaternions are handled by biorbd so yes :)

Reviewable status: 17 of 29 files reviewed, 5 unresolved discussions (waiting on @EveCharbie)


bioptim/examples/getting_started/pendulum.py line 131 at r2 (raw file):

Previously, EveCharbie (Eve Charbonneau) wrote…

revert?

revert to 300 ?


bioptim/examples/getting_started/pendulum.py line 160 at r2 (raw file):

Previously, EveCharbie (Eve Charbonneau) wrote…

make an enum

Ouin, I didn't want one more enum...


bioptim/examples/holonomic_constraints/two_pendulums.py line 229 at r2 (raw file):

Previously, EveCharbie (Eve Charbonneau) wrote…

Super clean!

Done. Thanks


bioptim/models/biorbd/biorbd_model.py line 810 at r2 (raw file):

Previously, EveCharbie (Eve Charbonneau) wrote…

Do we need this? If yes, uncomment please

Method deleted as I moved everything in viewer scripts


bioptim/models/biorbd/viewer_bioviz.py line 116 at r2 (raw file):

Previously, EveCharbie (Eve Charbonneau) wrote…

Was it like that? I think viz.exec() does this

Yes it was like that because, bioviz doesn't natively handle multiphase animations.

Copy link
Member

@pariterre pariterre left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 9 of 27 files at r1, 3 of 3 files at r3, all commit messages.
Reviewable status: all files reviewed, 10 unresolved discussions (waiting on @EveCharbie and @Ipuch)


bioptim/examples/getting_started/pendulum.py line 160 at r2 (raw file):

Previously, Ipuch (Pierre Puchaud) wrote…

Ouin, I didn't want one more enum...

viewer should not be in sol.animate


bioptim/examples/getting_started/pendulum.py line 77 at r3 (raw file):

    """

    bio_model = BiorbdModel(biorbd_model_path)

Add the viewer here (and not when calling sol.animate)


bioptim/examples/getting_started/pendulum.py line 162 at r3 (raw file):

    viewer = "bioviz"
    # viewer = "pyorerun"
    sol.animate(n_frames=0, viewer=viewer, show_now=True)
model = BiorbdModel(PATH, viewer=BiorbdModelViewer.BIOVIZ)
ocp = prepare_ocp(...)
...
ocp.nlp[0].model.viewer = BiorbdModelViewer.PYORERUN
sol.animate(n_frames=0, show_now=True)

bioptim/examples/torque_driven_ocp/example_multi_biorbd_model.py line 100 at r3 (raw file):

    # --- Animate results with pyorerun --- #
    sol.animate(viewer="pyorerun")

Update this


bioptim/optimization/solution/solution.py line 1225 at r3 (raw file):

        show_now: bool = True,
        show_tracked_markers: bool = False,
        viewer: str = "bioviz",

Move this to BiorbdModel interface


bioptim/optimization/solution/solution.py line 1264 at r3 (raw file):

        if viewer == "bioviz":
            return animate_with_bioviz_for_loop(self.ocp, self, show_now, show_tracked_markers, n_frames, **kwargs)

type(self.ocp.nlp[0].model).animate()

@pariterre pariterre changed the title [RTR] feat: pyorerun interfaced feat: pyorerun interfaced Jul 18, 2024
Copy link
Collaborator Author

@Ipuch Ipuch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: 25 of 30 files reviewed, 10 unresolved discussions (waiting on @EveCharbie and @pariterre)


bioptim/examples/getting_started/pendulum.py line 131 at r2 (raw file):

Previously, Ipuch (Pierre Puchaud) wrote…

revert to 300 ?

Done


bioptim/examples/getting_started/pendulum.py line 160 at r2 (raw file):

Previously, pariterre (Pariterre) wrote…

viewer should not be in sol.animate

gave up


bioptim/examples/getting_started/pendulum.py line 77 at r3 (raw file):

Previously, pariterre (Pariterre) wrote…

Add the viewer here (and not when calling sol.animate)

gave up


bioptim/examples/getting_started/pendulum.py line 162 at r3 (raw file):

Previously, pariterre (Pariterre) wrote…
model = BiorbdModel(PATH, viewer=BiorbdModelViewer.BIOVIZ)
ocp = prepare_ocp(...)
...
ocp.nlp[0].model.viewer = BiorbdModelViewer.PYORERUN
sol.animate(n_frames=0, show_now=True)

gave up


bioptim/examples/torque_driven_ocp/example_multi_biorbd_model.py line 100 at r3 (raw file):

Previously, pariterre (Pariterre) wrote…

Update this

Done


bioptim/optimization/solution/solution.py line 1225 at r3 (raw file):

Previously, pariterre (Pariterre) wrote…

Move this to BiorbdModel interface

Done


bioptim/optimization/solution/solution.py line 1264 at r3 (raw file):

Previously, pariterre (Pariterre) wrote…

type(self.ocp.nlp[0].model).animate()

Done

Copy link
Member

@pariterre pariterre left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm:

Reviewed 5 of 5 files at r4, all commit messages.
Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @EveCharbie)

@pariterre pariterre merged commit 0596f4a into pyomeca:master Jul 19, 2024
19 of 22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants