-
Notifications
You must be signed in to change notification settings - Fork 118
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
Fix bug in wave drag derivatives #432
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #432 +/- ##
==========================================
+ Coverage 93.92% 93.95% +0.02%
==========================================
Files 104 104
Lines 6472 6468 -4
==========================================
- Hits 6079 6077 -2
+ Misses 393 391 -2 ☔ View full report in Codecov by Sentry. |
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.
Good catch! Could you add a test that breaks before these changes and succeeds with them? Then it's good to go from me.
I was initially going to test this fix by enabling wave drag in the scaneagle test (which has a super low Mach number) and then adding a check_totals test. However, the totals there were fine. I kept the new totals derivatives test in there though as I think it's a good addition. I'm assuming the reason the scaneagle test didn't catch the bug is that the values in the partials arrays that get passed in to I managed to engineer a case like this by reducing the Mach number in |
Purpose
Wave drag is zero if the flight Mach number is below the critical Mach number. In this case, the partial derivatives should also be zero. However, the
compute_partials
method of the wave drag component simply does nothing to the partials arrays if M < Mcrit, leaving whatever values are already in the partial derivative arrays:This PR fixes this issue by explicitly zeroing out the wave drag partial derivatives at the start of
compute_partials
.I also made a few code quality improvements to remove some minor differences between code in
compute
andcompute_partials
that should be identical:mean_chords
incompute
andchords
incompute_partials
topanel_mid_chords
self.ka
instead of a hardcoded 0.95 incompute_partials
as is done incompute
compute
as is done incompute_partials
Expected time until merged
Type of change
Testing
Successfully converged optimisation that was previously failing. Also tested partial derivatives.
Before this change:
After this change:
If there is an existing test that checks our aero derivatives we should add a case for which M < Mcrit, I couldn't find any such tests though.
Checklist
flake8
andblack
to make sure the Python code adheres to PEP-8 and is consistently formattedfprettify
or C/C++ code withclang-format
as applicable