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

Multi-start module #555

Merged
merged 18 commits into from
Nov 2, 2022
Merged

Multi-start module #555

merged 18 commits into from
Nov 2, 2022

Conversation

EveCharbie
Copy link
Collaborator

@EveCharbie EveCharbie commented Oct 19, 2022

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? -> Issue Have a module for multistart OCP which use MPI #393
  • 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:

  1. Does your submission pass the tests (if not please explain why this is intended)?
  2. Did you write a proper documentation (docstrings and ReadMe)
  3. 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?

This change is Reviewable

Copy link
Collaborator

@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: 0 of 5 files reviewed, 1 unresolved discussion (waiting on @EveCharbie)


bioptim/optimization/multi_start.py line 67 at r1 (raw file):

                        else:
                            list_out += list_now
list_out = [instance for instance in product(*self.args_dict.values())]

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 5 of 5 files at r1, all commit messages.
Reviewable status: all files reviewed, 12 unresolved discussions (waiting on @EveCharbie)


bioptim/examples/getting_started/example_multistart.py line 113 at r1 (raw file):

def solve_ocp(args: list = None):

Remove this function

Code quote:

def solve_ocp(args: list = None):

bioptim/examples/getting_started/example_multistart.py line 130 at r1 (raw file):

    biorbd_model_path = args[0]
    final_time = args[1]

All this should be in prepare_multistart (as suggested later)

Code quote:

    biorbd_model_path = args[0]
    final_time = args[1]
    n_shooting = args[2]
    seed = args[3]

    ocp = prepare_ocp(biorbd_model_path, final_time, n_shooting)
    ocp.add_plot_penalty(CostType.ALL)
    sol = ocp.solve(Solver.IPOPT(show_online_optim=False))  # You cannot use show_online_optim with multi-start

bioptim/examples/getting_started/example_multistart.py line 136 at r1 (raw file):

    ocp = prepare_ocp(biorbd_model_path, final_time, n_shooting)
    ocp.add_plot_penalty(CostType.ALL)
    sol = ocp.solve(Solver.IPOPT(show_online_optim=False))  # You cannot use show_online_optim with multi-start

This should be called by the run()

Code quote:

sol = ocp.solve(Solver.IPOPT(show_online_optim=False))  # You cannot use show_online_optim with multi-start

bioptim/examples/getting_started/example_multistart.py line 137 at r1 (raw file):

    ocp.add_plot_penalty(CostType.ALL)
    sol = ocp.solve(Solver.IPOPT(show_online_optim=False))  # You cannot use show_online_optim with multi-start
    ocp.save(sol, f"solutions/pendulum_multi_start_random{seed}.bo", stand_alone=True)

The user can do this with the output from .run()

Code quote:

ocp.save(sol, f"solutions/pendulum_multi_start_random{seed}.bo", stand_alone=True)

bioptim/examples/getting_started/example_multistart.py line 144 at r1 (raw file):

        solve_ocp,
        n_random=10,
        n_pools=4,

Suggestion:

    ocp = solve_ocp(biorbd_model_path, final_time, n_shooting, seed=42)
    ocp.add_plot_penalty(CostType.ALL)
    return MultiStart(
        ocp,
        n_random=10,
        n_pools=4,
    )

bioptim/examples/getting_started/example_multistart.py line 155 at r1 (raw file):

        biorbd_model_path=["models/pendulum.bioMod"], final_time=[1], n_shooting=[30, 40, 50]
    )
    multi_start.run()

This should returns all the solution

multi_start.solve(solver=Solver.IPOPT(show_online_optim=False))

Suggestion:

multi_start.solve(Solver.IPOPT(show_online_optim=False))

bioptim/optimization/multi_start.py line 23 at r1 (raw file):

        n_random: int = 1,
        n_pools: int = 1,
        args_dict: dict = None,

Suggestion:

    def __init__(
        self,
        ocp,
        n_random: int = 1,
        n_pools: int = 1,
        **kwargs,
    ):

bioptim/optimization/multi_start.py line 38 at r1 (raw file):

        """

        self.solve_ocp = solve_ocp

self.ocp = ocp
self.solver = solver


bioptim/optimization/multi_start.py line 41 at r1 (raw file):

        self.n_random = n_random
        self.n_pools = n_pools
        self.args_dict = args_dict

Suggestion:

self.args_dict = **kwargs

bioptim/optimization/multi_start.py line 79 at r1 (raw file):

        """
        with Pool(self.n_pools) as p:
            p.map(self.solve_ocp, self.combined_args_to_list)

Suggestion:

p.map(self.ocp.solve, self.combined_args_to_list)

tests/test_global_getting_started.py line 1146 at r1 (raw file):

        biorbd_model_path=[bioptim_folder + "/models/pendulum.bioMod"], final_time=[1], n_shooting=[5, 10]
    )
    multi_start.run()

.solve()

Copy link
Collaborator Author

@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.

It is not exactly what you asked for, but I think it respects the idea the most possible

Reviewable status: 3 of 5 files reviewed, 12 unresolved discussions (waiting on @Ipuch and @pariterre)


bioptim/examples/getting_started/example_multistart.py line 113 at r1 (raw file):

Previously, pariterre (Pariterre) wrote…

Remove this function

Done.


bioptim/examples/getting_started/example_multistart.py line 130 at r1 (raw file):

Previously, pariterre (Pariterre) wrote…

All this should be in prepare_multistart (as suggested later)

Done.


bioptim/examples/getting_started/example_multistart.py line 136 at r1 (raw file):

Previously, pariterre (Pariterre) wrote…

This should be called by the run()

Done.


bioptim/examples/getting_started/example_multistart.py line 137 at r1 (raw file):

Previously, pariterre (Pariterre) wrote…

The user can do this with the output from .run()

Not possible to return sol because it cannot be pickled.


bioptim/examples/getting_started/example_multistart.py line 144 at r1 (raw file):

        solve_ocp,
        n_random=10,
        n_pools=4,

I cannot put ocp since the arguments would be fixed, but I put prepare_ocp


bioptim/examples/getting_started/example_multistart.py line 155 at r1 (raw file):

Previously, pariterre (Pariterre) wrote…

This should returns all the solution

multi_start.solve(solver=Solver.IPOPT(show_online_optim=False))

Done.


bioptim/optimization/multi_start.py line 23 at r1 (raw file):

        n_random: int = 1,
        n_pools: int = 1,
        args_dict: dict = None,

Done.


bioptim/optimization/multi_start.py line 38 at r1 (raw file):

Previously, pariterre (Pariterre) wrote…

self.ocp = ocp
self.solver = solver

Done.


bioptim/optimization/multi_start.py line 41 at r1 (raw file):

        self.n_random = n_random
        self.n_pools = n_pools
        self.args_dict = args_dict

Done.


bioptim/optimization/multi_start.py line 67 at r1 (raw file):

Previously, Ipuch (Pierre Puchaud) wrote…

from itertools import product
list_out = [instance for instance in product(*self.args_dict.values())]

Done


bioptim/optimization/multi_start.py line 79 at r1 (raw file):

        """
        with Pool(self.n_pools) as p:
            p.map(self.solve_ocp, self.combined_args_to_list)

Done.


tests/test_global_getting_started.py line 1146 at r1 (raw file):

Previously, pariterre (Pariterre) wrote…

.solve()

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.

Reviewed 4 of 4 files at r2, 1 of 1 files at r3.
Reviewable status: 4 of 6 files reviewed, 4 unresolved discussions (waiting on @EveCharbie and @Ipuch)


bioptim/examples/getting_started/example_multistart.py line 85 at r3 (raw file):

    u_bounds[1, :] = 0  # Prevent the model from actively rotate

    # To be changed when the issue on NoisedInitialGuess is fixed

TODO


bioptim/examples/getting_started/example_multistart.py line 143 at r3 (raw file):

        prepare_ocp,
        solver=Solver.IPOPT(show_online_optim=False),  # You cannot use show_online_optim with multi-start
        callback_function=save_results,

post_optimization_callback=save_results,

Code quote:

callback_function

bioptim/optimization/multi_start.py line 46 at r3 (raw file):

        self.prepare_ocp = prepare_ocp
        self.solver = solver
        self.callback_function = callback_function

post_optimization_callback

Code quote:

.callback_function = callback_function

@pariterre pariterre changed the title [RTR] Multi-start module [RTM_special] Multi-start module Oct 27, 2022
@pariterre
Copy link
Member

@EveCharbie I rebased this PR to integrate the new tests. Please use reset --hard (don't merge) to move to that PR if you need to push something later. Call me if needed

@pariterre
Copy link
Member

@EveCharbie
Tests fails so renaming to normal name!

@pariterre pariterre changed the title [RTM_special] Multi-start module Multi-start module Oct 28, 2022
@codecov
Copy link

codecov bot commented Nov 2, 2022

Codecov Report

Base: 83.76% // Head: 80.47% // Decreases project coverage by -3.28% ⚠️

Coverage data is based on head (8136cc5) compared to base (5027e98).
Patch coverage: 70.55% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #555      +/-   ##
==========================================
- Coverage   83.76%   80.47%   -3.29%     
==========================================
  Files          91       95       +4     
  Lines       10155    10382     +227     
==========================================
- Hits         8506     8355     -151     
- Misses       1649     2027     +378     
Impacted Files Coverage Δ
bioptim/examples/acados/static_arm.py 42.85% <0.00%> (ø)
...tim/examples/getting_started/example_multiphase.py 89.28% <0.00%> (-1.63%) ⬇️
bioptim/interfaces/sqp_interface.py 0.00% <0.00%> (ø)
bioptim/optimization/optimization_variable.py 85.29% <ø> (ø)
...tim/examples/getting_started/example_multistart.py 28.12% <28.12%> (ø)
bioptim/optimization/optimal_control_program.py 87.03% <50.00%> (-0.31%) ⬇️
bioptim/interfaces/solver_options.py 71.11% <61.45%> (-1.74%) ⬇️
bioptim/optimization/multi_start.py 88.46% <88.46%> (ø)
bioptim/optimization/solution.py 86.26% <88.88%> (-0.10%) ⬇️
bioptim/interfaces/interface_utils.py 90.38% <90.38%> (ø)
... and 10 more

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.
📢 Do you have feedback about the report comment? Let us know in this issue.

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 2 of 2 files at r4, 2 of 2 files at r6, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @Ipuch)

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:

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @Ipuch)

@pariterre pariterre merged commit a49807b into pyomeca:master Nov 2, 2022
@EveCharbie EveCharbie deleted the Multi-start branch May 30, 2023 07:14
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.

None yet

3 participants