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

Clean up besselj(nu,x) and add docs #28

Merged
merged 4 commits into from
Aug 4, 2022
Merged

Clean up besselj(nu,x) and add docs #28

merged 4 commits into from
Aug 4, 2022

Conversation

heltonmc
Copy link
Member

@heltonmc heltonmc commented Aug 2, 2022

I've added more docs to besselj and other internal methods.

I've also cleaned up the besselj implementation to be more consistent with bessely while removing several branches due to using the Hankel expansion instead of using either forward or backward recurrence. I think this helps with branch prediction and speeding up the overall routine. Accuracy is unchanged. I've also removed the loggamma implementation from SpecialFunctions.jl so this function is entirely in Julia now.

julia> @benchmark Bessels.besselj(y, x) setup=(x=rand()*300+20.0; y=rand()*300)
BenchmarkTools.Trial: 10000 samples with 963 evaluations.
 Range (min  max):   84.805 ns  580.607 ns  ┊ GC (min  max): 0.00%  0.00%
 Time  (median):     146.028 ns               ┊ GC (median):    0.00%
 Time  (mean ± σ):   178.862 ns ± 115.707 ns  ┊ GC (mean ± σ):  0.00% ± 0.00%

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

 Memory estimate: 0 bytes, allocs estimate: 0.

julia> @benchmark SpecialFunctions.besselj(y, x) setup=(x=rand()*300+20.0; y=rand()*300)
BenchmarkTools.Trial: 4171 samples with 535 evaluations.
 Range (min  max):  207.865 ns  7.374 μs  ┊ GC (min  max): 0.00%  0.00%
 Time  (median):       1.464 μs             ┊ GC (median):    0.00%
 Time  (mean ± σ):     2.237 μs ± 1.453 μs  ┊ GC (mean ± σ):  0.00% ± 0.00%

  ▄▅ ▂   ▁▄█▆▅▁▁    ▄▆▅▁▂▂▂▁  ▃▃▂     ▃▄▄▃▁▁                  ▂
  ████▃▃▅████████▃▃▅█████████▇███▇▃▄▁▇███████▅▆▆▅▃▁▁▁▃▄▃▁▇███ █
  208 ns       Histogram: log(frequency) by time      6.83 μs <

 Memory estimate: 16 bytes, allocs estimate: 1.

@codecov-commenter
Copy link

codecov-commenter commented Aug 2, 2022

Codecov Report

Merging #28 (4fd03bd) into master (a09b94d) will decrease coverage by 0.02%.
The diff coverage is 93.75%.

@@            Coverage Diff             @@
##           master      #28      +/-   ##
==========================================
- Coverage   99.06%   99.03%   -0.03%     
==========================================
  Files          14       14              
  Lines        1172     1145      -27     
==========================================
- Hits         1161     1134      -27     
  Misses         11       11              
Impacted Files Coverage Δ
src/asymptotics.jl 98.29% <ø> (+0.55%) ⬆️
src/bessely.jl 100.00% <ø> (ø)
src/besselj.jl 99.25% <92.85%> (-0.75%) ⬇️
src/U_polynomials.jl 99.28% <100.00%> (ø)

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

@heltonmc
Copy link
Member Author

heltonmc commented Aug 4, 2022

I'm going to merge this before #29 and deal with any conflicts.

@heltonmc heltonmc merged commit 527eb38 into master Aug 4, 2022
@heltonmc heltonmc deleted the besselj_docs branch August 4, 2022 00:19
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.

2 participants