Refactored the timeseries handling to be more general and less transcription-specific. #816
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…gup with the integration comp in ExplicitShooting...it needs an add_timeseries_output_configure that in turn reallocates the storage arrays.
…utputs. Colliding glob patterns will now result in a RuntimeError being raised during configure rather than a warning.
…e comp for gauss-lobatto
…ODE outputs out of the timeseries.
…back to dymos.static_output.
….its needed by the GaussLobatto version.
…lows for inputs to feed multiple outputs again.
…tup stack to configure.
…essarily not as accurate as the Pseudospectral one due to the equidistant time-spacing of nodes. Added some checks of SolveIVP rate outputs to the min time climb tests. Timeseries wildcard units are now handled correctly in the new system.
… order to more closely match the old warning. Removed some of the logic that checks for timeseries outputs not in the ODE in the RKIntegrationComp since it was being triggered by non-ode-outputs like states and controls.
…rror as a NameError instead of a RuntimeError.
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
robfalck
requested review from
naylor-b,
Kenneth-T-Moore and
kaushikponnapalli
naylor-b
requested changes
Aug 30, 2022
| @@ -339,6 +350,26 @@ def configure_interleave_comp(self, phase): | |||
| f'interleave_comp.col_values:state_rates:{state_name}', | |||
| src_indices=col_src_idxs) | |||
|
|
|||
| for timeseries_name, timeseries_options in phase._timeseries.items(): | |||
|
|
|||
| for ts_output_name, ts_output in phase._timeseries[timeseries_name]['outputs'].items(): | |||
There was a problem hiding this comment.
phase._timeseries[timeseries_name] --> timeseries_options
dymos/utils/introspection.py
Outdated
| output_options['units'] = output_meta['units'] | ||
|
|
||
| if not_found: | ||
| sorted_list = ' '.join(sorted(not_found)) |
There was a problem hiding this comment.
Maybe comma separate this list?
dymos/utils/introspection.py
Outdated
|
|
||
| for pattern in _patterns: | ||
| filtered.extend(fnmatch.filter(output_names, pattern)) | ||
| filtered = list(set(filtered)) # de-dupe |
There was a problem hiding this comment.
No need to convert set to a list here.
There was a problem hiding this comment.
old habits die hard
dymos/utils/introspection.py
Outdated
| for pattern in patterns: | ||
| filtered.update(fnmatch.filter(outputs, pattern)) | ||
| output_names = list(outputs.keys()) | ||
| filtered = [] |
There was a problem hiding this comment.
Better to just make filtered a set to begin with and call update on it with results from fnmatch instead of creating a list first and then converting to a set.
naylor-b
approved these changes
Aug 30, 2022
kaushikponnapalli
approved these changes
Aug 30, 2022
Removed unnecessary _get_rate_source_path implelmentation for SolveIVP.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Timeseries now have a two-step configuration process. In the first
configure_timeseries_output_glob_expansionmethod, dymos replaces any wildcard timeseries additions with their matching ODE outputs.In the second stage,
configure_timeseries_output_introspeciton, dymos performs introspection to determine default units, shape, and tags of the requested timeseries outputs.Internally, there's a change to the way "default" timeseries outputs are added (we call
add_timeseries_outputfor states, controls, time, and parameters). This keeps the code for introspecting timeseries outputs more general and just requires each transcription to implement_get_timeseries_var_source. See the implementation in the various transcriptions if you're curious.Added a new dymos-specific tag:
dymos.static_outputtells dymos that an output is not sized with num_nodes as its first dimension and therefore cannot be added to a timeseries or used as a boundary/path constraint. If a user wishes to constrain the quantity, they should apply OpenMDAO'sadd_constraintmethod to the relevant ODE path.The docs made references to the notion of "tandem phases" but they weren't explained. An example has been added.
Related Issues
Resolves Cleanup handling of timeseries outputs #714
Resolves Allow timeseries to provide interpolated rates of output variables. #765
Timeseries rate outputs have been extended to SolveIVP so that the user can get approximated rates from
simulate. Note: The equidistant time-spacing ofsimulatewill make these interpolated rates appear to be noisier in some points than they actually are.Timeseries rate outputs still are not implemented for ExplicitShooting.
Since timeseries outputs can be renamed and "special" variables are no longer treated differently, a user can now choose to add a timeseries output that renames a state. This can be helpful if two different phases use ODE's that employ different variable naming. A state can be renamed in the timeseries output to be whatever the user wishes.
The Commercial aircraft example was displaying automatically generated plots, but the path to those plots recently changed.
Backwards incompatibilities
The shape of
parameter_val:{param_name}is now(1,) + shape. Adding the first axis makes it possible to more easily connect the parameter into a timeseries with a slicer, such assrc_indices=om.slicer[gd.subset_num_nodes['all'], ...]While generally used only internally,
get_source_metadatawas changed to return a single dictionary of keys rather than a tuple. This will allow us to change what metadata may be returned without changing the structure of the call.New Dependencies
None