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

Deprecated SolveIVP transcription. simulate method now uses the ExplicitShooting transcription without derivatives. #898

Merged
merged 20 commits into from
Feb 6, 2023

Conversation

robfalck
Copy link
Contributor

@robfalck robfalck commented Jan 31, 2023

Summary

SolveIVP transcription is now deprecated.
Added SimulationPhase which is returned by phase.get_simulation_phase.

  • does not support optimization
  • currently only supports a single default timeseries

Changes to ExplicitShooting

  • num_segments, order, compressed, segment_ends are no longer deprecated. This makes the interface to create the transcription the same as the others. If using these options rather than grid, it will use a GaussLobattoGrid by default.
  • fixed a units bug in the related VandermondeControlInterpComp

Changes to grid_data

  • grid_data now treats transcription_order argument the same whether it is a scalar or a length-1 array.

Related Issues

Backwards incompatibilities

None

New Dependencies

None

…g transcription without derivative propagation instead of the SolveIVP transcription.

Fixed a get_cmap deprecation warning from matplotlib in the dymos plots.
…volve constraints and objectives have been overridden to inform the user that they are not to be applied to SimulationPhase.

Fixed an issue in pseudospectral_timeseries_output_comp which was causing it to fail if the intput and output grids were the same but the output subset was not 'all'.
Fixed a few indexing issues in grid_data.
ExplicitShooting phases are now allowed to simulate.
Un-deprecated num_segments, compressed, segment_ends, and order options to ExplicitShooting.
Defining the transcription using these options will now result in getting a GaussLobattoGrid by default.
Fixed a bug in ExplicitShooting handling of matrix-shaped states.
Fixed a units issue in TauComp.
@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@@ -83,7 +83,7 @@ def test_cruise_results_gl(self):
p.setup()

p['phase0.t_initial'] = 0.0
p['phase0.t_duration'] = 1.515132 * 3600.0
p['phase0.t_duration'] = 3600.0
Copy link
Contributor Author

Choose a reason for hiding this comment

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

initial value was out of bounds of the design variable.

@@ -181,7 +181,7 @@ def test_cruise_results_radau(self):
p.setup()

p['phase0.t_initial'] = 0.0
p['phase0.t_duration'] = 1.515132 * 3600.0
p['phase0.t_duration'] = 1.0 * 3600.0
Copy link
Contributor Author

Choose a reason for hiding this comment

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

initial value was out of bounds of the design variable

# # There is the nominal problem, the simulation problem, and a subproblem for each segment in the simulation.
self.assertEqual(len(report_subdirs), 12)
# There is the nominal problem, the simulation problem, and a subproblem for the simulation.
self.assertEqual(len(report_subdirs), 3)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

changed due to having fewer subproblems involved.


def test_ex_brachistochrone_radau_uncompressed(self):
ex_brachistochrone.SHOW_PLOTS = True
p = ex_brachistochrone.brachistochrone_min_time(transcription='radau-ps',
compressed=False)
self.run_asserts(p)
self.tearDown()
if os.path.exists('ex_brach_radau_uncompressed.db'):
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Handled by @use_tempdir

"\n",
"Since this is purely an integration with no controls to be determined, a single call to `run_model` will propagate the solution for us. There's no need to run a driver. Even the typical follow-up call to `traj.simulate` is unnecessary.\n",
"\n",
"Technically, we could even solve this using a single segment since segment spacing in the explicit shooting transcription determine the density of the control nodes, and there are no controls for this simulation.\n",
Copy link
Member

Choose a reason for hiding this comment

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

determine --> determines


# Copy over any simulation options from the simulate call. The fallback will be to
# Copy over any simulation options from the simulate call. The ffallback will be to
Copy link
Member

Choose a reason for hiding this comment

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

typo: ffallback

exp_out = p.model.phase0.simulate()
exp_out = p.model._get_subsystem('phase0').simulate()

time_comp = exp_out.model.phase0._get_subsystem(f'time')
Copy link
Member

Choose a reason for hiding this comment

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

fstrings are not needed in this block

@@ -32,6 +32,10 @@ class SolveIVP(TranscriptionBase):
Dictionary of optional arguments.
"""
def __init__(self, grid_data=None, **kwargs):
om.issue_warning('The SolveIVP transcription is deprecated. The simulate methods in Dymos now use a '
Copy link
Member

Choose a reason for hiding this comment

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

'Dymos now use a the ExplicitShooting...'

Copy link
Member

@johnjasa johnjasa left a comment

Choose a reason for hiding this comment

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

Looks great to me!

@@ -664,8 +665,8 @@ def _f_primal(self, t, x, theta):
----------
t : float
The current value of the integration variable.
y : np.array
The augmented state vector.
x : np.array
Copy link
Member

Choose a reason for hiding this comment

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

Some part of this docstring doesn't jive; right after the function definition, it says "where y is the augmented state vector" but now there's no y? And dtheta_dz isn't to be seen either? Maybe just updating the comments is all that's needed

@robfalck robfalck merged commit b0d6657 into OpenMDAO:master Feb 6, 2023
@robfalck robfalck deleted the i889 branch February 1, 2024 21:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants