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

Add option to select order of pre and post coupling subsystems to aerostructural scenario. #146

Merged
merged 4 commits into from
May 16, 2023

Conversation

kejacobson
Copy link
Collaborator

The aerostructural _mphys_scenario_setup function was getting long with these changes, so I broke it up into more functions for each phase.

Also moved fake_ldxfer to unit_test subdir where it should have already been.

@kejacobson kejacobson requested a review from anilyil May 11, 2023 16:08
Copy link
Collaborator

@anilyil anilyil left a comment

Choose a reason for hiding this comment

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

The addition of the options are fine by me. I have one comment on the implementation though; I would add the check in one location in setup to make sure the provided list of disciplines are the correct ones. Then when we are actually adding the pre or postprocessing subsystems, we can just loop over the option instead of having if checks for each discipline since it looks like only the discipline name is changing.

I can make this change if you want @kejacobson. Let me know what you think.

@kejacobson
Copy link
Collaborator Author

The addition of the options are fine by me. I have one comment on the implementation though; I would add the check in one location in setup to make sure the provided list of disciplines are the correct ones. Then when we are actually adding the pre or postprocessing subsystems, we can just loop over the option instead of having if checks for each discipline since it looks like only the discipline name is changing.

I can make this change if you want @kejacobson. Let me know what you think.

I can add this.

@kejacobson kejacobson requested a review from anilyil May 15, 2023 14:15
@kejacobson
Copy link
Collaborator Author

I think we still need to check which discipline in the loop in order to know which builder to use. I also like keeping the else with a ValueError in there just to be safe.

@anilyil anilyil merged commit 60a2ab8 into OpenMDAO:main May 16, 2023
@kejacobson kejacobson deleted the aerostruct_pre_post_order branch March 24, 2024 01:22
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.

2 participants