-
Notifications
You must be signed in to change notification settings - Fork 4
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
Calculate nodes and weights at GaussLegendre
construction time
#149
Conversation
Benchmark Results
Benchmark PlotsA plot of the benchmark results have been uploaded as an artifact to the workflow run for this PR. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #149 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 18 18
Lines 166 165 -1
=========================================
- Hits 166 165 -1 ☔ 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.
Nice! This can also be helpful if you want to integrate multiple functions even in potentially different geometries because you can reuse the rule object, i.e. also in this case you only need to compute the nodes and weight once.
Changes
nodes
andweights
as stored fields toGaussLegendre <: IntegrationRule
. These are generated at instantiation-time instead of atintegral
-time so that the results can be re-used.Discussion
The Gauss-Legendre nodes and weights are purely a function of order
n
, so I think it makes sense to just calculate them when constructing the rule itself. This will lead to some moderate performance benefits for rules used more than once, or where the integration occurs in some performance-sensitive user code block.Currently, FastGaussQuadrature.jl doesn't allow us to request nodes and weights in a particular
eltype
, and instead always returnsVector{Float64}
s. If that support ever materializes then I think it could make sense to add a parameterGaussLegendre{T}
to handle this, but for now I don't think it's necessary.