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

Move legacy code selection to input file #17

Merged
merged 29 commits into from
Jan 11, 2024
Merged

Conversation

jkirk5
Copy link
Contributor

@jkirk5 jkirk5 commented Dec 27, 2023

Summary

Which legacy code's methods can be used for a given input file is almost always limited to a single option based on which inputs were provided. Because of this, method selection was moved to a declaration in the input file to prevent mistakes when calling run_aviary.

  • Settings was added to the variable hierarchy to hold new "meta-level" switches, including setting equations of motion, mass method, and verbosity/debug level
  • New and existing enums were leveraged to define these settings. Enums were centralized and their usage throughout the code was standardized
  • Logic based on mission methods needed to be moved deeper down the interface levels. Now branching logic to determine default phase_info and anything that needs to know mission method is now in methods_for_level_2.py

Docs were updated to reflect this new, simplified interface. While updating the docs, I cleaned some of them up a bit.

Required future work:

  • More method flags for subsystems besides mass, which is how Aviary currently hardcodes subsystem creation
  • Utilize new verbosity flag throughout the code
  • Mild rework of methods_for_level_2.py may be helpful to cleanup the logic and make it easier for level 2 users to customize analysis. Such as simple and solved mission methods - they are technically sub-methods of height-energy and 2DOF, and treating them differently than full equations of motion may be needed.

Related Issues

Backwards incompatibilities

None

New Dependencies

None

@jkirk5 jkirk5 changed the title Cleanup Move legacy code selection to input file Dec 28, 2023
Copy link
Contributor

@crecine crecine left a comment

Choose a reason for hiding this comment

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

A few minor questions and comments for discussion

Comment on lines +158 to +159
user specifies. They could specify files to load and values to
replace here as well.
Copy link
Contributor

Choose a reason for hiding this comment

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

I know this isn't a result of this PR, but I'm confused by this sentence. Is this only referring to users that write their own load_inputs method?

Copy link
Contributor Author

@jkirk5 jkirk5 Jan 4, 2024

Choose a reason for hiding this comment

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

Yeah, I think so. The docstrings in methods_for_level_2 appear to be written for an audience of users writing their own level3 interface. Eventually we should have proper docstrings here and could move this kind of explanation to a level3 docs page.

aviary/interface/methods_for_level2.py Show resolved Hide resolved
aviary/interface/methods_for_level2.py Outdated Show resolved Hide resolved
aviary/interface/methods_for_level2.py Outdated Show resolved Hide resolved
aviary/interface/test/test_cmd_entry_points.py Outdated Show resolved Hide resolved
aviary/utils/test/test_Fortran_to_Aviary.py Outdated Show resolved Hide resolved
aviary/variable_info/enums.py Outdated Show resolved Hide resolved
aviary/variable_info/enums.py Show resolved Hide resolved
aviary/utils/Fortran_to_Aviary.py Outdated Show resolved Hide resolved
Copy link
Contributor

@jdgratz10 jdgratz10 left a comment

Choose a reason for hiding this comment

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

Love this change, it looks great! I did have a few questions/comments/requests, but in general I am in support of this PR. Good work!

aviary/docs/examples/modified_aircraft.csv Outdated Show resolved Hide resolved
aviary/docs/getting_started/onboarding_level1.ipynb Outdated Show resolved Hide resolved
aviary/docs/getting_started/onboarding_level2.ipynb Outdated Show resolved Hide resolved
aviary/interface/methods_for_level1.py Show resolved Hide resolved
aviary/interface/methods_for_level1.py Outdated Show resolved Hide resolved
aviary/variable_info/variables.py Show resolved Hide resolved
Copy link
Member

@johnjasa johnjasa left a comment

Choose a reason for hiding this comment

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

I pushed minor docstring changes directly to the branch; good to go from my end

@jkirk5 jkirk5 enabled auto-merge January 9, 2024 22:03
@jdgratz10 jdgratz10 dismissed their stale review January 11, 2024 15:26

Review is stale

@jkirk5 jkirk5 added this pull request to the merge queue Jan 11, 2024
Merged via the queue into OpenMDAO:main with commit ee4ccd9 Jan 11, 2024
6 checks passed
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.

Replace legacy code names with EOM names
4 participants