Skip to content
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

Internal speedup for calculating line opacities #167

Merged
merged 5 commits into from
Feb 12, 2024

Conversation

jvshields
Copy link
Contributor

@jvshields jvshields commented Feb 5, 2024

I changed some code to use vectorization and appropriate broadcasting to eliminate some for loops. For an 1000 angstrom wide spectrum with 0.01 angstrom wide bins, this reduces the calculation time by a factor of ~2. Considering that this is most computationally expensive part of the code, this offers a substantial speedup to stardis.

This change doesn't seem to show much in the benchmarks because we use a very small simulation in that case. This mainly gives significant speedups for large simulations that calculate many lines at many frequency bins.

Copy link

codecov bot commented Feb 5, 2024

Codecov Report

Attention: 2 lines in your changes are missing coverage. Please review.

Comparison is base (eba2b6d) 68.09% compared to head (1569426) 68.28%.

Files Patch % Lines
...adiation_field/opacities/opacities_solvers/base.py 77.77% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #167      +/-   ##
==========================================
+ Coverage   68.09%   68.28%   +0.18%     
==========================================
  Files          31       31              
  Lines        1163     1154       -9     
==========================================
- Hits          792      788       -4     
+ Misses        371      366       -5     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@jvshields jvshields added the benchmarks Trigger benchmarks to run on this pr label Feb 5, 2024
@tardis-bot
Copy link
Contributor

tardis-bot commented Feb 5, 2024

*beep* *bop*
Hi human,
I ran benchmarks as you asked comparing main (eba2b6d) and the latest commit (1569426).
Here are the logs produced by ASV.
Results can also be downloaded as artifacts here.
Significantly changed benchmarks:

All benchmarks:

All benchmarks:

     before           after         ratio
   [eba2b6dc]       [15694266]
        349±4ms          332±5ms     0.95  run_stardis.BenchmarkStardis.time_calc_alpha
    11.9±0.06ms      11.9±0.06ms     1.00  run_stardis.BenchmarkStardis.time_raytrace
     1.68±0.01s          1.66±0s     0.99  run_stardis.BenchmarkStardis.time_run_stardis

sonachitchyan
sonachitchyan previously approved these changes Feb 6, 2024
@jvshields jvshields marked this pull request as draft February 6, 2024 21:25
@jvshields jvshields marked this pull request as ready for review February 6, 2024 21:45
andrewfullard
andrewfullard previously approved these changes Feb 6, 2024
@DeerWhale DeerWhale merged commit 5fd5a8f into tardis-sn:main Feb 12, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
benchmarks Trigger benchmarks to run on this pr
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants