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

[RTR] Try to refactor a bit the initialisation of Solution #787

Merged
merged 3 commits into from
Nov 2, 2023

Conversation

Ipuch
Copy link
Collaborator

@Ipuch Ipuch commented Oct 30, 2023

This change is Reviewable

Ipuch added 2 commits October 30, 2023 16:21
refactoring init of solution refactoring
@codecov
Copy link

codecov bot commented Oct 30, 2023

Codecov Report

Attention: 174 lines in your changes are missing coverage. Please review.

Files Coverage Δ
bioptim/dynamics/integrator.py 95.97% <100.00%> (ø)
...amples/getting_started/example_variable_scaling.py 75.00% <ø> (ø)
bioptim/interfaces/stochastic_bio_model.py 52.94% <100.00%> (ø)
bioptim/limits/penalty.py 83.53% <100.00%> (ø)
bioptim/limits/penalty_option.py 83.04% <100.00%> (+0.04%) ⬆️
...ptim/optimization/receding_horizon_optimization.py 95.98% <100.00%> (ø)
bioptim/dynamics/dynamics_functions.py 84.29% <0.00%> (ø)
bioptim/interfaces/stochastic_biorbd_model.py 32.35% <33.33%> (ø)
bioptim/dynamics/ode_solver.py 85.20% <81.81%> (ø)
bioptim/optimization/optimal_control_program.py 85.95% <89.47%> (ø)
... and 4 more

📢 Thoughts on this report? Let us know!.

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


bioptim/optimization/solution.py line 504 at r1 (raw file):

                self._states["unscaled"],
                self._controls["unscaled"],
                [self.parameters for _ in range(self.ocp.n_phases)],  # artificially duplicate to use it in every phase

I do not understand the necessity of that nor that I think that it is a good idea?


bioptim/optimization/solution.py line 616 at r1 (raw file):

                self._states["unscaled"],
                self._controls["unscaled"],
                [self.parameters for _ in range(self.ocp.n_phases)],  # artificially duplicate to use it in every phase

I do not understand the necessity of that nor that I think that it is a good idea?


bioptim/optimization/solution.py line 649 at r1 (raw file):

                self._states["unscaled"],
                self._controls["unscaled"],
                [self.parameters for _ in range(self.ocp.n_phases)],  # artificially duplicate to use it in every phase

I do not understand the necessity of that nor that I think that it is a good idea?

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: 0 of 1 files reviewed, 3 unresolved discussions (waiting on @pariterre)


bioptim/optimization/solution.py line 504 at r1 (raw file):

Previously, pariterre (Pariterre) wrote…

I do not understand the necessity of that nor that I think that it is a good idea?

Done.


bioptim/optimization/solution.py line 616 at r1 (raw file):

Previously, pariterre (Pariterre) wrote…

I do not understand the necessity of that nor that I think that it is a good idea?

Done.


bioptim/optimization/solution.py line 649 at r1 (raw file):

Previously, pariterre (Pariterre) wrote…

I do not understand the necessity of that nor that I think that it is a good idea?

Done.

@Ipuch Ipuch changed the title Try to refactor a bit the initialisation of Solution [RTR] Try to refactor a bit the initialisation of Solution Oct 31, 2023
pariterre
pariterre previously approved these changes Nov 2, 2023
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 1 of 1 files at r2, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @Ipuch)

@Ipuch Ipuch dismissed pariterre’s stale review November 2, 2023 17:50

The merge-base changed after approval.

@pariterre pariterre merged commit 67a6ae9 into pyomeca:master Nov 2, 2023
20 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.

2 participants