FIX: changes Generic Motor exhaust velocity to cached property#497
Conversation
github-actions
bot
requested review from
Gui-FernandesBR,
MateusStano and
phmbressan
There was a problem hiding this comment.
Thank you for going through the journey to discover the bottleneck in this class. I was postponing for some time. Great finds!
Won't even start about snowball effect that the Function is currently generating that slows down the code so much. I will only say that the sooner this is fixed, the better.
This reverts commit ecf536d.
…icMotor to avoid lambdas
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #497 +/- ##
==========================================
- Coverage 70.95% 70.92% -0.04%
==========================================
Files 55 55
Lines 9262 9262
==========================================
- Hits 6572 6569 -3
- Misses 2690 2693 +3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Gui-FernandesBR
left a comment
Gui-FernandesBR
left a comment
There was a problem hiding this comment.
I swear that I tried but I honestly have no idea of how you discovered this error in such a chain of calls.
|
Before merging, do you see any possibility for new unit tests here? It's quite complicated because the error was only happening due to a "snowball" effect. |
MateusStano
deleted the
bug/generic-motor-exhaust-velocity-stalls-flight
branch
4 participants
Pull request type
Checklist
black rocketpy/ tests/) has passed locallypytest --runslow) have passed locallyCHANGELOG.mdhas been updated (if relevant)Current behavior
GenericMotor.exhaust_velocityis being decorated byfuncify_method, which makes it (a float essentially) into alambdadefinedFunction. This causes everything that depends on it algebraically to becomelambdadefinedFunctionsas well.More specifically, this contaminates
GenericMotor.mass_flow_rate, which in turn contaminatesGenericMotor.propellant_mass(which actually includes anintegral_functionof thelambdaGenericMotor.exhaust_velocity🫣). Well, in the end, this contaminatesRocket.total_mass, which is used extensively by theFlightclass during a simulation.In summary, this use of
funcify_methodcauses theFlightclass to take forever (literally) to run.New behavior
What can I say? Changing the decorator from
funcify_methodtocached_propertymakes the sun shine once again, solving everything.Breaking change