-
Notifications
You must be signed in to change notification settings - Fork 16
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
Speed up the airfoil drag calculations #118
base: main
Are you sure you want to change the base?
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #118 +/- ##
==========================================
- Coverage 79.14% 5.68% -73.47%
==========================================
Files 82 78 -4
Lines 13585 13552 -33
==========================================
- Hits 10752 770 -9982
- Misses 2833 12782 +9949 ☔ View full report in Codecov by Sentry. |
9ef62ab
to
5e1bcf6
Compare
0f2839c
to
43661ae
Compare
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 PR looks good to me. The aerodynamic functions in airfun.jl
in general are the least readable in all of TASOPT: they have essentially no comments or documentation and the variable names are not descriptive. Since the PR is not any worse in this regard than main, I believe it can be pulled in.
src/aero/surfcd.jl
Outdated
@@ -87,12 +81,12 @@ function surfcd2( | |||
Snorm = 0.0 | |||
ARe = wing.airsection.Re | |||
|
|||
for i = 1:n/2 | |||
frac = (float(i) - 0.5) / float(n / 2) | |||
for i = 1:n÷2 |
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.
It would be good to comment on why the integer division is used
1db0532
to
66082be
Compare
1. Stop searching for the right interval over and over and over. 2. Make functions type stable.
4555256
to
4a93e07
Compare
4a93e07
to
1b75d17
Compare
Got rid of a few more stray allocations that weren't needed. Wing cd calculations are allocation free now and significantly faster relative to main: julia> bench_surfcd2 = run(bench)
BenchmarkTools.Trial: 10000 samples with 5 evaluations.
Range (min … max): 8.225 μs … 1.750 ms ┊ GC (min … max): 0.00% … 99.12%
Time (median): 8.542 μs ┊ GC (median): 0.00%
Time (mean ± σ): 9.639 μs ± 29.016 μs ┊ GC (mean ± σ): 7.69% ± 2.78%
▄▇█▇▅▅▄▃▄▄▃▃▂▂▂▂▁▁▁▁▁▁ ▂
██████████████████████████▇▇▇▆▅▅▆▅▄▆▆▄▅▅▄▄▅▁▅▄▅▁▄▄▄▅▃▄▄▄▃▄ █
8.22 μs Histogram: log(frequency) by time 14.6 μs <
Memory estimate: 8.31 KiB, allocs estimate: 281. This PR: ---------------------------------------
surfcd2 (FORTRAN on MacPro M2 ~ 2.01 μs)
---------------------------------------
BenchmarkTools.Trial: 10000 samples with 5 evaluations.
Range (min … max): 1.633 μs … 12.058 μs ┊ GC (min … max): 0.00% … 0.00%
Time (median): 1.750 μs ┊ GC (median): 0.00%
Time (mean ± σ): 1.782 μs ± 342.426 ns ┊ GC (mean ± σ): 0.00% ± 0.00%
▂▂▂▄█
▃█████▇▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▁▂▂▂▂ ▃
1.63 μs Histogram: frequency by time 3.18 μs <
Memory estimate: 0 bytes, allocs estimate: 0. New documentation added too. |
Found a bug 🪲. The spline function had an error here: Line 36 in 277bc6d
|
This PR makes some improvements to the airfoil drag calculations to speed up the airsection calculations by roughly ~5x. The airfoil type wasn't really type stable/ it's field types weren't inferable by the compiler. Also the spline evaluations have been cleaned up a little and optimized for better performance.
On my M2 mac the performance of the airfoil section calculations are:
This PR: