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.

Sign up for GitHub

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

Jump to bottom

Aviary Options overhaul: Replace aviary_options with individual options on the analysis components. #553

Open
wants to merge 54 commits into
base: main
Choose a base branch
from
Open

Aviary Options overhaul: Replace aviary_options with individual options on the analysis components. #553

wants to merge 54 commits into from

Conversation

Kenneth-T-Moore
Copy link
Member

Summary

Major overhaul of options passing in Aviary.

  1. The option "aviary_inputs" has been removed from all components.
  2. Individual options have been added to all components, replacing the ones that used to be fished out of aviary_inputs.
  3. All options are now passed to the components using the OpenMDAO model options feature.
  4. Some options have units. OpenMDAO supports this, but the option's value becomes a tuple with a value and units string (e.g., (100.0 , 'ft') ).
  5. Added the setup_model_options method to sets the model options. This is used in tests and in level 3 models.

Due to this change, the "default value" and "types" fields in the variable metadata are now serving a dual role for type-checking in the AivaryValues user-facing API and in the OptionsDictionary.

  1. Some types related to propulsion were changed to include container types (list, ndarray, tuple).
  2. Some other default values were also added or tweaked.
  3. The FlapType and GaspEngineType enums were converted to IntEnums to match behavior of the other enums, which were changed in earlier PRs. This means we won't be able to set them by their string; this may not be ideal. The problem is that AviaryValues has its own special checking for Enums that OpenMDAO's OptionsDictionary does not have.

There are some matters that are left to possibly be addressed in the future:

  1. Many groups in Aviary still contain aviary_inputs, particular any group that calls builders.
  2. Builders still use aviary_inputs, which is an argument to build_*, so removing it at this level would be tricky.

Related Issues

Backwards incompatibilities

There shouldn't be any changes to the level 1 and level 2 user.

For level 3, a new helper function "setup_model_options" has been added to handle the model options. Level 3 users will need to call this before setup.

New Dependencies

None

@Kenneth-T-Moore
Initial implementation of model options.
915faaa
@Kenneth-T-Moore
Initial implementation of model options.
14196fc
@Kenneth-T-Moore
checkpoint, much of flops done
87328da
@Kenneth-T-Moore
Checkpoint. Fixed some tests. Need to move engine scaler out of optio…
2c49c77 
…ns on a separate story.
@Kenneth-T-Moore
checkpoint
2e848d8
@Kenneth-T-Moore
Fixed some more tests.
5515ec1
@Kenneth-T-Moore
Merge branch 'main' of github.com:OpenMDAO/Aviary into options
466b354
@Kenneth-T-Moore
All flops mass unit tests pass.
691b80a
@Kenneth-T-Moore
checkpoint
0b6740c
@Kenneth-T-Moore
Merge branch 'main' of github.com:OpenMDAO/Aviary into options
0f4df33
@Kenneth-T-Moore
Merged out, fixing incoming code.
4617bf9
@Kenneth-T-Moore
more tests fixed.
f044ec2
@Kenneth-T-Moore
Most unit tests pass in gasp
1eb1c79
@Kenneth-T-Moore
checkpoint
ba58368
@Kenneth-T-Moore
checkpoint
e1bc731
@Kenneth-T-Moore
handful of failures
157703c
@Kenneth-T-Moore
5 more tests pass
127d74f
@Kenneth-T-Moore
Merge branch 'main' of github.com:OpenMDAO/Aviary into options
28d8e3a
@Kenneth-T-Moore
Where we are.
0a8d8ba
@Kenneth-T-Moore
testing
3e892d4
@Kenneth-T-Moore
Checkpoint
69b6383
@Kenneth-T-Moore
Implemented option indexing for propulsion.
8936adc
@Kenneth-T-Moore
Multi engine tests pass
3fa38cb
@Kenneth-T-Moore
Another fix to the proulsion preprocessor.
ff408d5
@Kenneth-T-Moore
Most tests are passing.
4bc91c9
@Kenneth-T-Moore
fixing some docs
b4f142b
@Kenneth-T-Moore
one more doc
78f991a
@Kenneth-T-Moore
cleanup
0a4d84e
@Kenneth-T-Moore
merged out
ec2cca5
@Kenneth-T-Moore
Fix one option
148c454
crecine
crecine reviewed Oct 9, 2024
View reviewed changes
aviary/interface/methods_for_level2.py Outdated Show resolved Hide resolved
aviary/subsystems/aerodynamics/flops_based/mux_component.py Outdated Show resolved Hide resolved
aviary/subsystems/aerodynamics/gasp_based/flaps_model/meta_model.py Outdated Show resolved Hide resolved
aviary/subsystems/aerodynamics/gasp_based/gaspaero.py Outdated Show resolved Hide resolved
aviary/subsystems/aerodynamics/gasp_based/gaspaero.py Outdated Show resolved Hide resolved
aviary/subsystems/propulsion/test/test_engine_sizing.py Outdated Show resolved Hide resolved
aviary/subsystems/propulsion/test/test_propeller_performance.py
@@ -190,7 +190,8 @@ def setUp(self):

pp = prob.model.add_subsystem(
'pp',
PropellerPerformance(num_nodes=num_nodes, aviary_options=options),
PropellerPerformance(num_nodes=num_nodes,
aviary_options=options),
Copy link
Contributor

Choose a reason for hiding this comment

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

remove?

Copy link
Member Author

Choose a reason for hiding this comment

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

The interpolator builder still uses aviary_options. I am pretty sure we can remove it from there too, though that can be a nice compact follow-on.

aviary/utils/conflict_checks.py Outdated
@@ -3,13 +3,10 @@
from aviary.variable_info.variables import Aircraft


def check_fold_location_definition(inputs, options: AviaryValues):
def check_fold_location_definition(inputs, choose_fold_location, has_strut):
Copy link
Contributor

Choose a reason for hiding this comment

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

these might need defaults, these conflict checks were originally intended to have a standardized interface.

Copy link
Member Author

Choose a reason for hiding this comment

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

What default values would you recommend for the 2 options?

Copy link
Contributor

Choose a reason for hiding this comment

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

originally, all of the conflict checks (half dozen or so) all took inputs and options as positional args. Since this is the only one left, maybe it would be fine to just remove inputs and have it just take the variables it needs

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.

aviary/utils/preprocessors.py Outdated Show resolved Hide resolved
aviary/utils/test/test_aviary_values.py Outdated Show resolved Hide resolved
@jkirk5 jkirk5 self-requested a review October 21, 2024 17:34
@crecine crecine requested review from ehariton and removed request for jkirk5 October 30, 2024 20:15
crecine
crecine approved these changes Oct 30, 2024
View reviewed changes
aviary/utils/test/test_aviary_values.py
self.assertTrue(vals.get_val(Aircraft.Engine.TYPE)
== GASPEngineType.TURBOJET)
except:
self.fail('Expecting to be able to set the value of an Enum from an int.')
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this should be string

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@crecine crecine crecine approved these changes

@ehariton ehariton Awaiting requested review from ehariton

At least 2 approving reviews are required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Options Traceability
2 participants
@Kenneth-T-Moore @crecine