Aviary Options overhaul: Replace aviary_options with individual options on the analysis components. #553
Conversation
…ns on a separate story.
aviary/subsystems/aerodynamics/gasp_based/flaps_model/meta_model.py
Outdated
Show resolved
Hide resolved
| @@ -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), | |||
There was a problem hiding this comment.
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): | |||
There was a problem hiding this comment.
these might need defaults, these conflict checks were originally intended to have a standardized interface.
There was a problem hiding this comment.
What default values would you recommend for the 2 options?
There was a problem hiding this comment.
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
| 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.') |
There was a problem hiding this comment.
I think this should be string
| desc='fraction of (scaled) engine mass used to calculate additional propulsion ' | ||
| 'system mass added to engine control and starter mass, or used to ' | ||
| 'calculate engine installation mass', | ||
| types=(float, int, list, np.ndarray), |
There was a problem hiding this comment.
Adding list as an expected type here could potentially mess up a lot of multi-engine stuff.
Currently metadata type is used by propulsion to see what the type should be for a singular engine, which is important to distinguish the difference between an array which represents all engines, or an array that only represents one engine (such as for WING_LOCATIONS). If type is updated like this, it is impossible for the preprocessor to know what the user intended.
It also makes it difficult to type check arrays (you could give this var a list of strings and it would pass type checks now)
There was a problem hiding this comment.
For more detail, engines shouldn't be getting passed the vectorized values for all engines (an engine doesn't need to know the options of another engine). The use of aviary_options in the build_mission() and build_premission() methods of EngineModel can probably get removed, since they are instead given options during init.
| elif isinstance(value, str): | ||
| return getattr(enum_class, value) | ||
|
|
||
| elif isinstance(value, list): |
There was a problem hiding this comment.
or numpy array? I could see situations like preprocessors defaulting to sticking things in a numpy array
| if isinstance(value, Enum): | ||
| return value | ||
|
|
||
| elif isinstance(value, int): | ||
| return enum_class(value) | ||
|
|
||
| elif isinstance(value, str): | ||
| return getattr(enum_class, value) |
There was a problem hiding this comment.
I think these checks are actually redundant - feeding value in to enum_class will correctly return the Enum value in all three scenarios, and invalid values can be caught with an except - e.g. VERBOSITY(VERBOSITY.BRIEF) and VERBOSITY(1) will both return VERBOSITY.BRIEF
Summary
Major overhaul of options passing in Aviary.
(100.0 , 'ft')).setup_model_optionsmethod 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.
typesrelated to propulsion were changed to include container types (list, ndarray, tuple).There are some matters that are left to possibly be addressed in the future:
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