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

Implement benchmarking with AirspeedVelocity.jl #119

Merged
merged 27 commits into from
Oct 25, 2024
Merged

Implement benchmarking with AirspeedVelocity.jl #119

merged 27 commits into from
Oct 25, 2024

Conversation

mikeingold
Copy link
Collaborator

@mikeingold mikeingold commented Oct 24, 2024

Changes

  • Adds a CI Action (duplicate of this one) that uses AirspeedVelocity.jl to run benchmarks for every new PR
  • Adds a benchmark script with a suite containing 14 @benchmark entries, testing integrals of Segment and Sphere (surface), jacobian, and differential.

Closes #21

@mikeingold mikeingold added the enhancement New feature or request label Oct 24, 2024
Copy link

codecov bot commented Oct 24, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.75%. Comparing base (6c8cd1f) to head (eedc533).
Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #119   +/-   ##
=======================================
  Coverage   95.75%   95.75%           
=======================================
  Files          17       17           
  Lines         259      259           
=======================================
  Hits          248      248           
  Misses         11       11           

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

benchmark/benchmarks.jl Outdated Show resolved Hide resolved
@mikeingold mikeingold marked this pull request as ready for review October 25, 2024 00:10
@mikeingold
Copy link
Collaborator Author

@JoshuaLampert, this seems to work well on my system and should provide us with benchmarks to quantify performance impacts of each PR. The Action script is taken straight from AirspeedVelocity.jl; I don't see any blatant red flags, but you're more knowledgeable in that domain than I am.

Copy link
Member

@JoshuaLampert JoshuaLampert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks quite neat! I would like to see the action already running for this PR. Normally GitHub already runs newly added actions already in the PR, where they are added. So let's see if fixing the pull_request_target branch runs the action.

.github/workflows/AirspeedVelocity.yml Outdated Show resolved Hide resolved
.github/workflows/AirspeedVelocity.yml Outdated Show resolved Hide resolved
benchmark/benchmarks.jl Outdated Show resolved Hide resolved
@JoshuaLampert
Copy link
Member

It still doesn't run. I'm not sure why.

@JoshuaLampert
Copy link
Member

Ahh, there we go. Now it runs, but fails. Let me fix that quickly.

Copy link
Member

@JoshuaLampert JoshuaLampert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There were some old versions.

.github/workflows/AirspeedVelocity.yml Outdated Show resolved Hide resolved
.github/workflows/AirspeedVelocity.yml Outdated Show resolved Hide resolved
.github/workflows/AirspeedVelocity.yml Outdated Show resolved Hide resolved
.github/workflows/AirspeedVelocity.yml Outdated Show resolved Hide resolved
.github/workflows/AirspeedVelocity.yml Outdated Show resolved Hide resolved
.github/workflows/AirspeedVelocity.yml Outdated Show resolved Hide resolved
Copy link
Contributor

github-actions bot commented Oct 25, 2024

Benchmark Results

main eedc533... main/eedc533858dc6f...
Differentials/Differential 0.765 ± 0.066 μs 0.764 ± 0.063 μs 1
Differentials/Jacobian 0.668 ± 0.026 μs 0.683 ± 0.026 μs 0.979
Integrals/Meshes.Segment/Scalar GaussKronrod 4.4 ± 0.22 μs 4.4 ± 0.21 μs 1
Integrals/Meshes.Segment/Scalar GaussLegendre 0.075 ± 0.00051 ms 0.0742 ± 0.00049 ms 1.01
Integrals/Meshes.Segment/Scalar HAdaptiveCubature 7.93 ± 0.034 μs 7.97 ± 0.05 μs 0.996
Integrals/Meshes.Segment/Vector GaussKronrod 6.6 ± 0.075 μs 6.64 ± 0.097 μs 0.994
Integrals/Meshes.Segment/Vector GaussLegendre 0.0881 ± 0.0027 ms 0.0907 ± 0.0044 ms 0.972
Integrals/Meshes.Segment/Vector HAdaptiveCubature 10.9 ± 0.13 μs 10.9 ± 0.14 μs 0.996
Integrals/Meshes.Sphere/Scalar GaussKronrod 0.195 ± 0.00051 ms 0.198 ± 0.00066 ms 0.984
Integrals/Meshes.Sphere/Scalar GaussLegendre 16.8 ± 1.2 ms 16.9 ± 1.2 ms 0.993
Integrals/Meshes.Sphere/Scalar HAdaptiveCubature 0.19 ± 0.0021 ms 0.19 ± 0.0023 ms 1
Integrals/Meshes.Sphere/Vector GaussKronrod 0.237 ms 0.232 ms 1.02
Integrals/Meshes.Sphere/Vector GaussLegendre 18.9 ± 0.46 ms 18.9 ± 0.43 ms 1
Integrals/Meshes.Sphere/Vector HAdaptiveCubature 0.255 ± 0.0057 ms 0.261 ± 0.0089 ms 0.979
time_to_load 1.59 ± 0.0094 s 1.58 ± 0.011 s 1.01

Benchmark Plots

A plot of the benchmark results have been uploaded as an artifact to the workflow run for this PR.
Go to "Actions"->"Benchmark a pull request"->[the most recent run]->"Artifacts" (at the bottom).

@JoshuaLampert
Copy link
Member

JoshuaLampert commented Oct 25, 2024

Everything runs fine now. Also uploading the plots works and updating the comment after a new push works nicely.
(Sorry for all the noise I made in this PR)

@mikeingold
Copy link
Collaborator Author

Thanks for getting this working!

@mikeingold mikeingold merged commit 8938720 into main Oct 25, 2024
12 checks passed
@mikeingold mikeingold deleted the benchmarks branch October 25, 2024 12:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement benchmark test suite
2 participants