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

avoid integrand evaluations at endpoints #91

Merged
merged 5 commits into from
Nov 20, 2023
Merged

Conversation

lxvm
Copy link
Contributor

@lxvm lxvm commented Sep 23, 2023

This pr concerns issues #86, #82, and #39, and addresses errors arising from evaluation of the integrand at the endpoints of the interval. This pr handles the endpoints by:

  • throwing a DomainError if the roundoff happens on an initial segment
  • silently returning the best integral and error estimate if the roundoff happens during refinement

I think it would be better to warn the user of the following situations in which quadgk silently returns a possibly unconverged result

  • endpoint roundoff error
  • maxevals reached

Should we do this and add return codes?

@codecov
Copy link

codecov bot commented Sep 23, 2023

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.04% 🎉

Comparison is base (10bc1a1) 98.16% compared to head (173eff3) 98.20%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master      #91      +/-   ##
==========================================
+ Coverage   98.16%   98.20%   +0.04%     
==========================================
  Files           6        6              
  Lines         599      614      +15     
==========================================
+ Hits          588      603      +15     
  Misses         11       11              
Files Changed Coverage Δ
src/adapt.jl 96.99% <100.00%> (+0.32%) ⬆️
src/batch.jl 99.17% <100.00%> (+0.01%) ⬆️

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

@stevengj
Copy link
Member

stevengj commented Sep 23, 2023

Should we do this and add return codes?

The problem is that adding return codes to quadgk is breaking.

Of course, you can always tell from the error estimate — if the final error estimate is greater than the requested tolerance, you know that it must have returned early.

@stevengj
Copy link
Member

This introduces a type-instability into evalrule. Does that affect performance?

@lxvm
Copy link
Contributor Author

lxvm commented Sep 23, 2023

Of course, you can always tell from the error estimate — if the final error estimate is greater than the requested tolerance, you know that it must have returned early.

Right, that's a good point

This introduces a type-instability into evalrule. Does that affect performance?

Thanks for pointing this out, so I refactored a bit and put the type instability into the refine routine.

As for performance, the endpoint check has an overhead of about 3 ns, which should be negligible. Here is a benchmark:

master

julia> using BenchmarkTools

julia> using QuadGK

julia> @benchmark quadgk(x -> x, 0, 1) # zero-cost integrand, no subdivisions
BenchmarkTools.Trial: 10000 samples with 993 evaluations.
 Range (min  max):  34.597 ns  133.580 ns  ┊ GC (min  max): 0.00%  0.00%
 Time  (median):     35.161 ns               ┊ GC (median):    0.00%
 Time  (mean ± σ):   36.762 ns ±   6.744 ns  ┊ GC (mean ± σ):  0.00% ± 0.00%

  █▅▆                                                          ▁
  ███▆▅▅▃▅▅▆▇▇█▆▆▆▄▆▆▆▆▆▆▆▅▆█▇▅▄▇█▅▅▄▆▅▅▄▆▅▄▄▄▅▅▄▁▅▅▅▅▆▆▅▅█▅▄▃ █
  34.6 ns       Histogram: log(frequency) by time      72.5 ns <

 Memory estimate: 0 bytes, allocs estimate: 0.

julia> @benchmark quadgk(x -> sin(100x), 0, 1) # low-cost integrand with subdivisions
BenchmarkTools.Trial: 10000 samples with 1 evaluation.
 Range (min  max):  11.684 μs  63.263 μs  ┊ GC (min  max): 0.00%  0.00%
 Time  (median):     11.858 μs              ┊ GC (median):    0.00%
 Time  (mean ± σ):   12.611 μs ±  1.834 μs  ┊ GC (mean ± σ):  0.00% ± 0.00%

  ▆█▅▂             ▃▆▆▄▃▁             ▁▂▁                     ▂
  █████▆▅▄▁▁▁▁▁▁▄▁▁███████▇▆▆▇▇▇▇▅▆▆▄▅███▇▇▆▄▁▁▃▁▃▃▄▅▄▁▃▅▁▄▄▅ █
  11.7 μs      Histogram: log(frequency) by time      16.9 μs <

 Memory estimate: 1.69 KiB, allocs estimate: 3.

this pr

julia> using BenchmarkTools

julia> using QuadGK

julia> @benchmark quadgk(x -> x, 0, 1) # zero-cost integrand, no subdivisions
BenchmarkTools.Trial: 10000 samples with 992 evaluations.
 Range (min  max):  37.661 ns  132.844 ns  ┊ GC (min  max): 0.00%  0.00%
 Time  (median):     37.885 ns               ┊ GC (median):    0.00%
 Time  (mean ± σ):   39.574 ns ±   7.290 ns  ┊ GC (mean ± σ):  0.00% ± 0.00%

  █  ▁                                                         ▁
  █▇▃█▃▃▃▄▅▄▄▅▅▅▄▂▃▄▂▂▄▆▆▅▅▄▅▅▅▄▅▄▅▅▄▄▄▅▅▅▅▄▄▃▅▅▅▆▆▇▆▆▆▅▅▄▄▄▄▄ █
  37.7 ns       Histogram: log(frequency) by time      75.2 ns <

 Memory estimate: 0 bytes, allocs estimate: 0.

julia> @benchmark quadgk(x -> sin(100x), 0, 1) # low-cost integrand with subdivisions
BenchmarkTools.Trial: 10000 samples with 1 evaluation.
 Range (min  max):  11.616 μs  79.306 μs  ┊ GC (min  max): 0.00%  0.00%
 Time  (median):     11.986 μs              ┊ GC (median):    0.00%
 Time  (mean ± σ):   12.831 μs ±  2.304 μs  ┊ GC (mean ± σ):  0.00% ± 0.00%

  ▄██▄▃     ▂▆▇▄▃▁      ▂▂                                    ▂
  █████▇▁▃▃▄██████▇███▅▇███▇▆▆▅▆▆▄▄▅▄▃▄▅▆▇▇▅▆▃▄▄▄▁▄▁▄▄▄▃▁▄▃▁▄ █
  11.6 μs      Histogram: log(frequency) by time      20.5 μs <

 Memory estimate: 1.69 KiB, allocs estimate: 3.

@lxvm lxvm marked this pull request as ready for review September 23, 2023 23:38
@stevengj stevengj merged commit 17242d3 into JuliaMath:master Nov 20, 2023
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants