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

Updated parameter introspection and traj parameters #964

Merged
merged 11 commits into from
Aug 23, 2023

Conversation

robfalck
Copy link
Contributor

@robfalck robfalck commented Aug 14, 2023

Summary

Trajectory parameters now pull introspection data from constituent phases even under MPI.
Phase parameters now allow for a mixture of static and dynamic targets.

Deprecated static_target in favor of static_targets which is a set of those targets which are static.**

Previously option static_target was an all-or-nothing setting for a parameter.
Now option static_targets is a set of targets to which the connection is treated as a static one (not sized with the number of nodes in the phase.
Dymos will use introspection to attempt to determine which targets are static.
static_targets may still be provided as an "all-or-nothing" True or False option.

Changes to internal method _get_targets_metadata

The method now returns a dictionary keyed by target path and associated with metadata (shape, units, value, tags) of the given target.

Added new introspection method _get_common_metadata

This method will accept targets and a metadata key, and then return the value of the given metadata key if it is shared across all targets. This is useful when trying to infer the units or shape of a parameter.

Changes to ODEEvaluationGroup

Under the ExplicitShooting transcription, ODEEvaluationGroup is required to perform its own introspection since its configure method occurs before that of the parent phase. This group is now passed copies of the time, state, control, and parameter options dictionaries of the parent phase, so that results of its introspection will not conflict with those of the parent phase.

Related Issues

Backwards incompatibilities

None

New Dependencies

None

_get_targets_metadata now returns metadata on a per target basis, and determining the appropriate shape of a control, state, or
parameter is done outside of the function.
brachistochrone and some other tests are working but a lot of work is left to be done.
this should allow us to introspect the appropriate default shape and value for parameters and other variables.
…rameter has no targets since it may be used as a rate source.
@coveralls
Copy link

coveralls commented Aug 15, 2023

Coverage Status

coverage: 92.674% (+0.1%) from 92.569% when pulling 103b9c2 on robfalck:traj_params_update into 1aca42a on OpenMDAO:master.

"""
phase_param_options = {}
for phs in self.phases._subsystems_myproc:
phase_param_options.update({phs.name: phs.parameter_options})
Copy link
Member

Choose a reason for hiding this comment

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

Creating a temporary dict just to call update isn't necessary. Could just do phase_param_options[phs.name] = phs.parameter_options

phase_param_options.update({phs.name: phs.parameter_options})

if self.comm.size > 1:
data = self.comm.gather(phase_param_options, root=0)
Copy link
Member

Choose a reason for hiding this comment

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

I think you need an allgather here. Otherwise only rank 0 contains the full data and all other procs only contain their local data.


# Check that all targets have the same shape.
static_shapes = {}
dynamic_shapes = {}
Copy link
Member

Choose a reason for hiding this comment

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

Do you need 2 different dicts here since you combine them together later anyway?

ValueError
ValueError is raised if the targets do not all have the same metadata value.
"""
meta_set = {meta[metadata_key] for tgt, meta in targets.items()}
Copy link
Member

Choose a reason for hiding this comment

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

You only need the metadata here, not the target name, so could just use targets.values()

else:
introspected_shape = None

if options['shape'] in {_unspecified, None}:
Copy link
Member

Choose a reason for hiding this comment

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

You use {_unspecified, None} or {None, _unspecified} in quite a few places in dymos. Would it be worth it to define a global constant with that value to avoid creating all of those temporary sets?

@robfalck robfalck requested a review from naylor-b August 23, 2023 19:23
@robfalck robfalck merged commit 99ec21e into OpenMDAO:master Aug 23, 2023
9 checks passed
@robfalck robfalck deleted the traj_params_update branch August 14, 2024 15:30
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.

Parameter issues.
3 participants