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

Permit src_indices for an input in both group.promotes and group.connect provided that both are flat. #1709

Closed
wants to merge 15 commits into from

Conversation

Kenneth-T-Moore
Copy link
Member

Summary

Permit src_indices for an input in both group.promotes and group.connect provided that both are flat. A new cascaded src_indices is computed for the connection.

Related Issues

Backwards incompatibilities

None

New Dependencies

None

@project-bot project-bot bot added this to In progress in OpenMDAO Dev [Read only] Sep 30, 2020
OpenMDAO Dev [Read only] automation moved this from In progress to Reviewer approved Sep 30, 2020
Copy link
Member

@naylor-b naylor-b left a comment

Choose a reason for hiding this comment

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

Are we really sure that we want to allow users to do this? If we happen to see src_indices specified in multiple places, how can we be sure of the intent? Did they intend to do this sort of nested src_indices thing, or did they just make a mistake?

@Kenneth-T-Moore
Copy link
Member Author

I don't know. There is some risk, but the risk stems from the user not knowing what they are connecting to, and if you are using src_indices, you should know the shape of your target.

Alternative is to make the user create an adapter component.

@robfalck @JustinSGray

Copy link
Member

@naylor-b naylor-b left a comment

Choose a reason for hiding this comment

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

Or maybe add another metadata entry like 'allow_nested_src_indices' or something so we know the intent.

@Kenneth-T-Moore
Copy link
Member Author

After discussion, we determined a better way to do this inside a dymos model.

OpenMDAO Dev [Read only] automation moved this from Reviewer approved to Unmerged PR Sep 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
3 participants