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.

Sign up for GitHub

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

Jump to bottom

Use more efficient parametric transformation for Tetrahedron #159

Merged
merged 6 commits into from
Dec 27, 2024
Merged

Use more efficient parametric transformation for Tetrahedron #159

merged 6 commits into from
Dec 27, 2024
+34 −19

Conversation

mikeingold
Copy link
Collaborator

@mikeingold mikeingold commented Dec 27, 2024 •
edited
Loading

Changed

  • Implement a new parametric transformation for Tetrahedron that first locates the desired point in barycentric space, requiring only one call to the original parametric function.

@codecov Codecov
Copy link

codecov bot commented Dec 27, 2024 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (16835af) to head (1866636).
Report is 1 commits behind head on main.

Additional details and impacted files 
@@            Coverage Diff            @@
##              main      #159   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           18        18           
  Lines          184       184           
=========================================
  Hits           184       184

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

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Dec 27, 2024 •
edited
Loading

Benchmark Results

main 1866636... main/18666363604b0d...
Differentials/Differential 0.203 ± 0.0011 μs 0.203 ± 0.002 μs 1
Differentials/Jacobian 0.166 ± 0.001 μs 0.167 ± 0.0011 μs 0.994
Integrals/Segment/Scalar GaussKronrod 0.842 ± 0.0081 μs 0.833 ± 0.0078 μs 1.01
Integrals/Segment/Scalar GaussLegendre 1.62 ± 0.0071 μs 1.64 ± 0.009 μs 0.993
Integrals/Segment/Scalar HAdaptiveCubature 1.04 ± 0.14 μs 1.02 ± 0.1 μs 1.02
Integrals/Segment/Vector GaussKronrod 3.02 ± 0.062 μs 3.03 ± 0.071 μs 0.996
Integrals/Segment/Vector GaussLegendre 17.6 ± 0.55 μs 17.6 ± 0.54 μs 1
Integrals/Segment/Vector HAdaptiveCubature 3.91 ± 0.11 μs 3.93 ± 0.12 μs 0.994
Integrals/Sphere/Scalar GaussKronrod 0.0727 ± 0.00076 ms 0.0728 ± 0.0013 ms 0.998
Integrals/Sphere/Scalar GaussLegendre 1.81 ± 0.0091 ms 1.81 ± 0.0084 ms 1
Integrals/Sphere/Scalar HAdaptiveCubature 0.0469 ± 8.1e-05 ms 0.0469 ± 0.0001 ms 1
Integrals/Sphere/Vector GaussKronrod 0.107 ± 0.0013 ms 0.106 ± 0.0015 ms 1.01
Integrals/Sphere/Vector GaussLegendre 3.25 ± 0.084 ms 3.29 ± 0.069 ms 0.988
Integrals/Sphere/Vector HAdaptiveCubature 0.098 ± 0.0014 ms 0.0977 ± 0.0014 ms 1
Rules/GaussLegendre 22.1 ± 0.59 μs 22 ± 0.54 μs 1
Specializations/Scalar GaussLegendre/BezierCurve 0.252 ± 0.00028 ms 0.25 ± 0.00024 ms 1.01
Specializations/Scalar GaussLegendre/Line 7.07 ± 0.068 μs 7.04 ± 0.056 μs 1
Specializations/Scalar GaussLegendre/Plane 0.738 ± 0.0024 ms 0.74 ± 0.0023 ms 0.998
Specializations/Scalar GaussLegendre/Ray 6 ± 0.049 μs 5.98 ± 0.055 μs 1
Specializations/Scalar GaussLegendre/Rope 0.0516 ± 0.00021 ms 0.0516 ± 0.0002 ms 1
Specializations/Scalar GaussLegendre/Tetrahedron 0.281 ± 0.0021 s 0.155 ± 0.0024 s 1.82
Specializations/Scalar GaussLegendre/Triangle 0.597 ± 0.0031 ms 0.714 ± 0.024 ms 0.836
time_to_load 1.54 ± 0.022 s 1.53 ± 0.027 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).

@mikeingold mikeingold self-assigned this Dec 27, 2024
JoshuaLampert
JoshuaLampert reviewed Dec 27, 2024
View reviewed changes
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.

Thanks! This is a nice speedup of 80%. Codecov complains about the docstring-like comments. Could we maybe turn those into normal comments # to satisfy codecov? Documenter.jl doesn't pick up these docstrings anyway. But if you prefer the status quo I would also be fine.

src/specializations/Tetrahedron.jl Outdated Show resolved Hide resolved
src/specializations/Triangle.jl Outdated Show resolved Hide resolved
@mikeingold mikeingold marked this pull request as ready for review December 27, 2024 14:40
JoshuaLampert
JoshuaLampert approved these changes Dec 27, 2024
View reviewed changes
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.

LGTM!

@mikeingold
Copy link
Collaborator Author

Thanks for the review!

@mikeingold mikeingold merged commit 8de05a1 into main Dec 27, 2024
12 checks passed
@mikeingold mikeingold deleted the tetra branch December 27, 2024 20:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JoshuaLampert JoshuaLampert JoshuaLampert approved these changes

Assignees

@mikeingold mikeingold

Labels
refactoring
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Possible to speed up _parametric(::Tetrahedron)
2 participants
@mikeingold @JoshuaLampert