-
Notifications
You must be signed in to change notification settings - Fork 140
Unify subsystem addition and solver control across all of our ODEs. #1284
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
base: main
Are you sure you want to change the base?
Changes from all commits
bf7c843
ef45c51
b205779
6c0ef22
e5b72e1
bbdc466
1d530f5
e1a8278
7cabe7f
5e1922e
194552e
e084dd3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,8 @@ | ||
| import openmdao.api as om | ||
|
|
||
| from aviary.subsystems.aerodynamics.aerodynamics_builder import AerodynamicsBuilder | ||
| from aviary.subsystems.atmosphere.atmosphere import Atmosphere | ||
| from aviary.subsystems.propulsion.propulsion_builder import PropulsionBuilder | ||
| from aviary.utils.aviary_values import AviaryValues | ||
| from aviary.variable_info.variable_meta_data import CoreMetaData | ||
|
|
||
|
|
@@ -52,31 +54,36 @@ def add_atmosphere(self, **kwargs): | |
| promotes=['*'], | ||
| ) | ||
|
|
||
| def add_subsystems(self, solver_group=None): | ||
| def add_subsystems_and_solver( | ||
| self, solver_sub=None, couple_propulsion=False, couple_aero=False, aero_solver_sub=False | ||
| ): | ||
| """ | ||
| Adds all specified subsystems to ODE in their own group. | ||
| Adds all specified subsystems to this ODE. Subsystems that need a solver due to coupling | ||
| are instead added to a group called "solver_sub". | ||
|
|
||
| Parameters | ||
| ---------- | ||
| solver_group : om.Group | ||
| If not None, subsystems that require a solver (subsystem.needs_mission_solver() == True) | ||
| are placed inside solver_group. | ||
|
|
||
| If None, all subsystems are added to BaseODE regardless of if they request a solver. | ||
| TODO add solver compatibility to all ODEs | ||
|
|
||
| solver_sub: None or om.Group | ||
| Pre-created group to add the solver. | ||
| couple_propulsion : bool | ||
| When True, the ODE couples with any propulsion subsystems via a throttle to commanded | ||
| thrust balance. | ||
| couple_aero : bool | ||
| When True, the ODE couples with any aerodynamics subsystems via a force balance. | ||
| aero_solver_sub : None or om.Group | ||
| Some ODEs (like solved 2DOF) place the aerodynamics and propulsion cycles in separate | ||
| groups. When this is specified, the aerodynamics subsystem is placed in this sub. | ||
| Returns | ||
| ------- | ||
| use_mission_solver : bool | ||
| Flag that communicates that one or more subsystem requests to be placed inside a solver | ||
| (independent of the needs of an individual ODE's setup) | ||
| om.Group | ||
| Target group for the ODE. This will be self unless a solver is needed, in which case it | ||
| will be solver_sub. | ||
| """ | ||
| nn = self.options['num_nodes'] | ||
| aviary_options = self.options['aviary_options'] | ||
| all_subsystems = self.options['subsystems'] | ||
| all_subsystem_options = self.options['subsystem_options'] | ||
| user_options = self.options['user_options'] | ||
| use_mission_solver = False | ||
|
|
||
| for subsystem in all_subsystems: | ||
| # check if subsystem_options has entry for a subsystem of this name | ||
|
|
@@ -100,9 +107,36 @@ def add_subsystems(self, solver_group=None): | |
| subsystem_options=subsystem_options, | ||
| ) | ||
|
|
||
| if needs_solver and solver_group is not None: | ||
| target = solver_group | ||
| use_mission_solver = True | ||
| # ODE couples with propulsion. | ||
| if couple_propulsion and isinstance(subsystem, PropulsionBuilder): | ||
| needs_solver = True | ||
| elif couple_aero and isinstance(subsystem, AerodynamicsBuilder): | ||
| needs_solver = True | ||
|
|
||
| if needs_solver: | ||
| if solver_sub is None: | ||
| solver_sub = self.add_subsystem('solver_sub', om.Group(), promotes=['*']) | ||
| solver_sub.options['auto_order'] = True | ||
|
|
||
| solver_sub.nonlinear_solver = om.NewtonSolver( | ||
| solve_subsystems=True, | ||
| atol=1.0e-10, | ||
| rtol=1.0e-10, | ||
| err_on_non_converge=True, | ||
| iprint=2, | ||
| ) | ||
| solver_sub.nonlinear_solver.linesearch = om.BoundsEnforceLS() | ||
|
|
||
| solver_sub.linear_solver = om.DirectSolver(assemble_jac=True) | ||
|
|
||
| if ( | ||
| aero_solver_sub | ||
| and couple_aero | ||
| and isinstance(subsystem, AerodynamicsBuilder) | ||
| ): | ||
| target = aero_solver_sub | ||
| else: | ||
| target = solver_sub | ||
|
|
||
| mission_in = subsystem.mission_inputs( | ||
| aviary_inputs=aviary_options, | ||
|
|
@@ -121,4 +155,4 @@ def add_subsystems(self, solver_group=None): | |
| promotes_outputs=mission_out, | ||
| ) | ||
|
|
||
| return use_mission_solver | ||
| return solver_sub if solver_sub else self | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is returning the group useful? I feel like it will cause confusion, because an easy mistaken interpretation of this function is that it returns the correctly configured group of subsystems and solvers they then need to add to the ODE. If users need the group object for some reason it already got added to the ODE so they should probably use OM interface to access that info in their ODE.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. My suggestion is simply no return statement at all here
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. After we return it, some of the ODEs will add the ode or a balance comp to the solver group. I could modify it back so that each ODE is responsible for creating the Group and passing it into the add_subsystems, which is what it was doing originally. It is just some duplicated code.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh ok that makes sense! I guess it is better how you have it here |
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This argument seems hyper-specialized for a specific ODE, which I think disqualifies it as useful to include in
BaseODE. Is a better approach to have these ODEs have their own definitions for the method? I don't want to keep having to add toBaseODEas we add additional equations of motion that also have special rules for how they want things set up.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The solved 2dof is the only ODE that creates two different solver groups.