-
Notifications
You must be signed in to change notification settings - Fork 110
Remove requirement that engines compute max thrust and other values #946
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?
Conversation
AI was consulted to brainstorm approaches
|
I am currently testing out removing max value computation from the turboprop model. This is pretty tricky and may have exposed some broken edge cases in the current implementation. Please do not merge yet!! |
This task is now complete! There is still additional cleanup and other small improvements that could be made (mostly within the turboprop component itself), but the core concept works |
| engine_group.add_subsystem( | ||
| 'fixed_max_throttles', fixed_throttles, promotes_outputs=['*'] | ||
| 'fixed_max_throttles', | ||
| fixed_throttles, # , promotes_outputs=['*'] |
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.
Can remove all of the commented out promotes statements.
| mission_solver = np.any([shp_solver, gearbox_solver, prop_solver]) | ||
| return mission_solver | ||
|
|
||
| # BUG if using multiple custom subsystems that happen to share a kwarg but need different values, |
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.
Would like to hear more about this bug to understand what is going on.
Summary
Maximum thrust at a given flight condition is required to compute excess specific power for mission. Previously, EngineModels were required to output max thrust, which is a large overhead for users and difficult to clearly communicate through documentation and the API.
The propulsion subsystem was upgraded to check if an EngineModel provides max values (via a new flag,
self.compute_max_values, defaulted to False), and if not creates a duplicate copy of the mission components that are fed a throttle & hybrid throttle of 1 instead. No outputs are promoted from this duplicate engine to avoid promotion/connection issues. Thrust outputted by the duplicate engine is directly connected to the engine mux component's max thrust input.Cleaned up some pieces of propulsion code including formatting, bugfixes, TODO's, etc. as I found them and was able to resolve on the spot.
Disclosure: Generative AI was used to draft coding approaches to handle the parallel engine groups, I ended up not using its suggestions but I did keep a few minor fixes it tried. Code in
propulsion_mission.py: lines 6 and 49 (plus indenting the lines I already wrote immediately following that line) were added by AI. The Cursor IDE was used, which does not disclose which LLM models it uses.Related Issues
Backwards incompatibilities
EngineModels that already compute max thrust can continue to do so as long as they add the
compute_max_valuesflag asTruein their models. If they don't, their component will get duplicated, which theoretically shouldn't break anything but will be very inefficient.New Dependencies
None