trajectory results report now automatically generated by run_problem #918
Conversation
…shows the timeseries plots and parameter values for that phase.
Added timeseries_options to Phase to allow the option of including control rates and state rates in the timeseries outputs (non-inclusion is the default). Added use_prefix as an option in timeseries_options to remove the prefixed variable class from the timeseries outputs (e.g. 'states:x' is now just 'x', 'controls:u' is now just 'u').
…'use_timeseries_prefix'] Removed 'include_states' and 'include_controls' timeseries options for now.
Fixed a bug in ExplicitShooting when connecting the controls to the continuity comp. Removed some commented lines from introspection.py.
…rders for grid refinement, for now.
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
…tory is non existent.
…is not available.
…re throwing design variables out of bounds.
…eries_reports_update
timeseries plot units should now plot on the "largest" unit found in the data.
There was a problem hiding this comment.
Nice, I really like this PR. Besides the trajectory results report, the "small" options changes are very helpful. I can't wait to not have timeseries in my models (no offense).
I've looked at the bokeh-produced html output for a few cases and I really dig it. I have small nitpicks/questions about the presentation:
- is it possible increase the dpi of the text rendering for the axis labels and legends? I had to ctrl+ zoom in to see the names better on my screen, but they were pretty pixelated.
- did you consider having 1- or 3-wide plots instead of 2-wide? I'm thinking about best monitor usage and I could where having different options might be beneficial, but I don't know how tough that would be to handle
- some long names for plotted quantities sometimes fall off the edge of the y-axis label, like for
control_rates:power_code_rate2 (unitless). If you say "hey that's just too long" that's fine, but maybe there's a quick way to scale the plot y-label or allow for different sized fonts? Not a huge deal
Overall great stuff! I really love when the majority of lines changed are for tests. That's a great sign. Thanks for continuing to improve Dymos, Rob!
| desc='The plot library used to generate output plots for Dymos.') | ||
|
|
||
| options.declare('notebook_mode', default=False, types=bool, | ||
| desc='If True, provide notebook-enhanced plots and outputs.') | ||
|
|
||
| options.declare('use_timeseries_prefix', default=True, types=bool, | ||
| desc='If True, prefix timeseries outputs with the variable type for states, times, controls,' |
There was a problem hiding this comment.
Does it make sense to acknowledge here that the default behavior will change later on?
There was a problem hiding this comment.
Regarding the 1st and 3rd bullets, I will look into the DPI. It's likely a font size issue but this will also impact your 3rd bullet point. That issue can be alleviated by turning off the prefixes.
Regarding the number of columns in the plots, I will see if that can be done via the UI so the user can change it on the fly. I will do that as a subsequent PR.
I'm planning on a relatively short time frame for the release of the next version, so I won't be making a note about the default behavior changing.
dymos/phase/options.py
Outdated
|
|
||
| class PhaseTimeseriesOptionsDictionary(om.OptionsDictionary): | ||
| """ | ||
| An OptionsDictionary for phase options related to timeseries.. |
There was a problem hiding this comment.
get that second period out of here!
| @@ -544,26 +545,31 @@ def _update_linkage_options_configure(self, linkage_options): | |||
| units[i] = phases[i].time_options['units'] | |||
| shapes[i] = (1,) | |||
| elif classes[i] == 'state': | |||
| sources[i] = f'timeseries.states:{vars[i]}' | |||
| prefix = 'states:' if dymos_options['use_timeseries_prefix'] else '' | |||
There was a problem hiding this comment.
Nice slick way of handling this prefix name logic throughout
|
Oh, and the two changes to default behavior you mention in the PR text. Is that a big deal? Will users have to change behavior or see different behavior than expected and be confused? I'm not sure how common those would come up, even. |
The only users who may notice this are those who have built up some post processing machinery that relies on the current timeseries name format. I'll include big bold warnings in the next version about this change. |
Summary
This is an update to the plot generation in dymos.
Now as single html file will be created for each trajectory.
It will contain:
These plots rely on bokeh and a dummy html file will be generated if it cannot be imported.
New options have been added:
dymos.options['use_timeseries_prefix'] = Truewill cause timeseries outputs of states, times, controls and parameters to be prefixed by the variable type (e.g.timeseries.states:x). This is the current default behavior of Dymos.dymos.options['use_timeseries_prefix'] = Falsewill remove the prefix (e.g.timeseries.x).In a future version of dymos we will set the default value to False, but leave this option in place for users who want to recover the existing behavior.
Phases now have
timeseries_optionsthat control what is added to the timeseries by default. The currently available options are:phase.timeseries_options['include_state_rates']currently defaults to True and is necessary for grid refinement.phase.timeseries_options['include_control_rates']defaults to False (this is a change to current behavior). Timeseries control rates can be provided by requesting them explicitly or setting this option to True.phase.timeseries_options['include_t_phase']defaults to False (this is a change to current behavior).time_phasecan be added to the timeseries by requesting it explicitly or by setting this option to True.Related Issues
Backwards incompatibilities
The dymos options module is now _options. This was causing a namespace collision.
Users can now access the global dymos options as
dm.options, for instance:New Dependencies
bokeh is still optional but is required for the trajectory report to be generated.