[RTR] Fix sol.integrate shooting.single continuity

[Kev1CO]

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:

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 

[Kev1CO]

Last blocking tests are the dimension comparation between states from sol.integrate and time from sol.stepwise_time because sol.stepwise_time as still redundant nodes compared to sol.integrate states.
Should I give an argument to sol.stepwise_time(remove_redundant=True) and adjust the dimension here or in the tests compare time and states from sol.integrate(return_time=True)?

[pariterre]

Reviewed 1 of 2 files at r1, 1 of 1 files at r2, all commit messages.
Reviewable status: all files reviewed, 9 unresolved discussions (waiting on @Kev1CO)

---

bioptim/interfaces/solve_ivp_interface.py line 51 at r2 (raw file):

    y = []
    control_type = nlp.control_type
    integration_time_vector = []

All the changes are no longer needed anymore

---

bioptim/optimization/solution/solution.py line 240 at r2 (raw file):

                [
                    len(s) != len(all_ns) if p != 3 and len(sol[p + 1].keys()) != 0 else False
                    for p, s in enumerate(sol[:1])

Remove the list comprehension so it is easier to read is_right_size

---

bioptim/optimization/solution/solution.py line 300 at r2 (raw file):

                    vector = np.concatenate((vector, np.repeat(ss[key].init, 1)[:, np.newaxis]))
                    # TODO : ask pariterre about this modification (the previous version did not enable the parameters
                    #  to be taken into consideration for each phase)

Okay

---

bioptim/optimization/solution/solution.py line 705 at r2 (raw file):

        integrator: SolutionIntegrator = SolutionIntegrator.OCP,
        to_merge: SolutionMerge | list[SolutionMerge] = None,
        return_time: bool = False,

add duplicated_times: bool = True

Please add docstring

Please add duplicated_times in stepwise_time

---

bioptim/optimization/solution/solution.py line 755 at r2 (raw file):

                out[p][key] = [None] * nlp.n_states_nodes
                for ns, sol_ns in enumerate(integrated_sol):
                    out[p][key][ns] = sol_ns[nlp.states[key].index, :]

        out[p] = {}
            for key in nlp.states.keys():
                out[p][key] = [None] * nlp.n_states_nodes
                for ns, sol_ns in enumerate(integrated_sol):
                    if duplicated_times:
                        out[p][key][ns] = sol_ns[nlp.states[key].index, :]
                    else:
                        out[p][key][ns] = sol_ns[nlp.states[key].index, :-1]
                        
        if to_merge:
            out = SolutionData.from_unscaled(self.ocp, out, "x").to_dict(to_merge=to_merge, scaled=False)

        if return_time:
            return self.stepwise_time(to_merge=to_merge, duplicated_times=duplicated_times), out
        else:
            return out if len(out) > 1 else out[0]

---

bioptim/optimization/solution/solution.py line 759 at r2 (raw file):

        if to_merge:
            out = SolutionData.from_unscaled(self.ocp, out, "x").to_dict(to_merge=to_merge, scaled=False)
            time_vector, out = self._remove_integrated_duplicates(time_vector, out, shooting_type, to_merge)

remove this method

---

bioptim/optimization/solution/solution.py line 759 at r2 (raw file):

        if to_merge:
            out = SolutionData.from_unscaled(self.ocp, out, "x").to_dict(to_merge=to_merge, scaled=False)
            time_vector, out = self._remove_integrated_duplicates(time_vector, out, shooting_type, to_merge)

add if return_time so it is not performed if time is not returned anyway

---

bioptim/optimization/solution/solution.py line 846 at r2 (raw file):

            to_merge == SolutionMerge.NODES or SolutionMerge.NODES in to_merge
        ):
            if (isinstance(to_merge, list) and SolutionMerge.PHASES in to_merge) or to_merge == SolutionMerge.PHASES:

Remove the test to_merge isinstance but add:

    if to_merge is None or isinstance(to_merge, SolutionMerge):  
        to_merge = [to_merge]

at the beggining of integrate

EDIT:
This method is probably useless

---

bioptim/optimization/solution/solution.py line 915 at r2 (raw file):

        unscaled: list = [None] * len(self.ocp.nlp)
        for p, nlp in enumerate(self.ocp.nlp):
            phase_times, integrated_sol = solve_ivp_interface(

solve_ivp_interface should not be returned anymore

[Kev1CO]

Do not know how to fix last failed tests

[Kev1CO]

Reviewable status: 0 of 3 files reviewed, 9 unresolved discussions (waiting on @pariterre)

---

bioptim/optimization/solution/solution.py line 240 at r2 (raw file):

Previously, pariterre (Pariterre) wrote…

Remove the list comprehension so it is easier to read is_right_size

Done.

---

bioptim/optimization/solution/solution.py line 300 at r2 (raw file):

Previously, pariterre (Pariterre) wrote…

Okay

Done.

---

bioptim/optimization/solution/solution.py line 705 at r2 (raw file):

Previously, pariterre (Pariterre) wrote…

add duplicated_times: bool = True

Please add docstring

Please add duplicated_times in stepwise_time

Done.

---

bioptim/optimization/solution/solution.py line 755 at r2 (raw file):

Previously, pariterre (Pariterre) wrote…

        out[p] = {}
            for key in nlp.states.keys():
                out[p][key] = [None] * nlp.n_states_nodes
                for ns, sol_ns in enumerate(integrated_sol):
                    if duplicated_times:
                        out[p][key][ns] = sol_ns[nlp.states[key].index, :]
                    else:
                        out[p][key][ns] = sol_ns[nlp.states[key].index, :-1]
                        
        if to_merge:
            out = SolutionData.from_unscaled(self.ocp, out, "x").to_dict(to_merge=to_merge, scaled=False)

        if return_time:
            return self.stepwise_time(to_merge=to_merge, duplicated_times=duplicated_times), out
        else:
            return out if len(out) > 1 else out[0]

Done.

---

bioptim/optimization/solution/solution.py line 759 at r2 (raw file):

Previously, pariterre (Pariterre) wrote…

remove this method

Done.

---

bioptim/optimization/solution/solution.py line 759 at r2 (raw file):

Previously, pariterre (Pariterre) wrote…

add if return_time so it is not performed if time is not returned anyway

Done.

---

bioptim/optimization/solution/solution.py line 846 at r2 (raw file):

Previously, pariterre (Pariterre) wrote…

Remove the test to_merge isinstance but add:

    if to_merge is None or isinstance(to_merge, SolutionMerge):  
        to_merge = [to_merge]

at the beggining of integrate

EDIT:
This method is probably useless

Done.

---

bioptim/optimization/solution/solution.py line 915 at r2 (raw file):

Previously, pariterre (Pariterre) wrote…

solve_ivp_interface should not be returned anymore

Done.

---

bioptim/interfaces/solve_ivp_interface.py line 51 at r2 (raw file):

Previously, pariterre (Pariterre) wrote…

All the changes are no longer needed anymore

Done.

[Kev1CO]

When returning time sol.integrate(return_time=True), should the time vector be concatenated in one array (array([0, 0.1, 0.2])) instead of keeping its initial shape of array([[0],
[0.1],
[0.2]])

[pariterre]

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

---

bioptim/optimization/solution/solution.py line 705 at r2 (raw file):

Previously, Kev1CO wrote…

Done.

Docstring was not added

---

bioptim/optimization/solution/solution.py line 489 at r4 (raw file):

            for i in range(len(times)):
                for j in range(len(times[i])):
                    keep_condition = times[i][j].shape[0] == 1 and i == len(times) - 1

When does this happens?

---

bioptim/optimization/solution/solution.py line 765 at r4 (raw file):

                        out[p][key][ns] = sol_ns[nlp.states[key].index, :]
                    else:
                        duplicated_times_condition = p == len(self.ocp.nlp) - 1 and ns == nlp.ns

When is that not the case?

---

tests/shard4/test_simulate.py line 613 at r4 (raw file):

        "integrator": integrator,
        "to_merge": [SolutionMerge.PHASES, SolutionMerge.NODES],
    }

it works but this is a bit confusing.
just call sol.integrate(shooting_type=shooting, integrator=integrator, to_merge=[SolutionMerge.PHASES, SolutionMerge.NODES], duplicated_times=False, return_time=True)

[codecov]

Codecov Report

Attention: Patch coverage is 82.85714% with 6 lines in your changes are missing coverage. Please review.

Project coverage is 78.50%. Comparing base (7494252) to head (4cda59c).
Report is 100 commits behind head on master.

Files	Patch %	Lines
bioptim/optimization/solution/solution.py	82.85%	6 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #859      +/-   ##
==========================================
+ Coverage   78.25%   78.50%   +0.25%     
==========================================
  Files         139      141       +2     
  Lines       16228    16309      +81     
==========================================
+ Hits        12699    12804     +105     
+ Misses       3529     3505      -24     

Flag	Coverage Δ
unittests	78.50% <82.85%> (+0.25%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[Kev1CO]

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

---

bioptim/optimization/solution/solution.py line 705 at r2 (raw file):

Previously, pariterre (Pariterre) wrote…

Docstring was not added

Done.

---

bioptim/optimization/solution/solution.py line 489 at r4 (raw file):

Previously, pariterre (Pariterre) wrote…

When does this happens?

at last phase integration node
therefore, we can not remove it like other last node phases

---

bioptim/optimization/solution/solution.py line 765 at r4 (raw file):

Previously, pariterre (Pariterre) wrote…

When is that not the case?

same than time duplicates
not removing last phase last node for each key

---

tests/shard4/test_simulate.py line 613 at r4 (raw file):

Previously, pariterre (Pariterre) wrote…

it works but this is a bit confusing.
just call sol.integrate(shooting_type=shooting, integrator=integrator, to_merge=[SolutionMerge.PHASES, SolutionMerge.NODES], duplicated_times=False, return_time=True)

Done.

[pariterre]

Reviewed 2 of 2 files at r5, 1 of 1 files at r6.
Reviewable status: 3 of 4 files reviewed, 1 unresolved discussion (waiting on @Kev1CO)

---

bioptim/optimization/solution/solution.py line 798 at r6 (raw file):

        if return_time:
            time_vector = np.concatenate(self.stepwise_time(to_merge=to_merge, duplicated_times=duplicated_times))

This won't work for not merged values

[Kev1CO]

Reviewable status: 2 of 4 files reviewed, 1 unresolved discussion (waiting on @pariterre)

---

bioptim/optimization/solution/solution.py line 798 at r6 (raw file):

Previously, pariterre (Pariterre) wrote…

This won't work for not merged values

Done.

[pariterre]

Reviewed 2 of 2 files at r8, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @Kev1CO)