Skip to content

Conversation

@curio-sitas
Copy link
Contributor

  • Reformulated calculations to avoid multiplications
  • Use of precise √π provided by IrrationalConstants.jl
  • Duplicates of functions for real inputs (faster calculations)
  • Added tests for arbitrary values

All above was suggested in JuliaMath/SpecialFunctions.jl#407

Tests are passing and the benchmark against QuadGK.jl is preserved

Added tests
Reformulated functions, and added lighter ones for real numbers as suggested in JuliaMath/SpecialFunctions.jl#407
@devmotion
Copy link
Contributor

Duplicates of functions for real inputs (faster calculations)

They also return Real numbers now instead of Complex numbers with zero imaginary part.

On my computer, with the changes in this PR the benchmarks are roughly twice as fast for Real inputs:

On master

julia> using FresnelIntegrals, QuadGK, BenchmarkTools

julia> @btime fresnelc(1.8)
  196.234 ns (0 allocations: 0 bytes)
0.3336329272215571 + 0.0im

julia> @btime quadgk(t->cos*t^2/2),0,1.8)
  950.158 ns (36 allocations: 1.02 KiB)
(0.33363292722155713, 1.348964606684433e-9)

This PR

julia> using FresnelIntegrals, BenchmarkTools, QuadGK

julia> @btime fresnelc(1.8)
  95.555 ns (0 allocations: 0 bytes)
0.33363292722155713

julia> @btime quadgk(t->cos*t^2/2),0,1.8)
  988.727 ns (40 allocations: 1.08 KiB)
(0.33363292722155713, 1.348964606684433e-9)

Another improvement of this PR is that it avoids duplicate computations in fresnel (similar to how sincos does not actually call both sin and cos separately). That leads to a significant performance improvement (~ 4x):

On master

julia> @btime fresnel(1.8)
  389.158 ns (0 allocations: 0 bytes)
(0.3336329272215571 + 0.0im, 0.4509387692675832 + 0.0im)

This PR

julia> @btime fresnel(1.8)
  96.117 ns (0 allocations: 0 bytes)
(0.33363292722155713, 0.4509387692675829)

For completeness, here's a benchmark with complex inputs which only benefit from the improvements to fresnel (~ 2x faster):

On master

julia> @btime fresnelc($(complex(1.8)))
  196.545 ns (0 allocations: 0 bytes)
0.3336329272215571 + 0.0im

julia> @btime fresnel($(complex(1.8)))
  389.356 ns (0 allocations: 0 bytes)
(0.3336329272215571 + 0.0im, 0.4509387692675832 + 0.0im)

This PR

julia> @btime fresnelc($(complex(1.8)))
  197.625 ns (0 allocations: 0 bytes)
0.33363292722155713 + 1.3877787807814457e-17im

julia> @btime fresnel($(complex(1.8)))
  198.163 ns (0 allocations: 0 bytes)
(0.33363292722155713 + 1.3877787807814457e-17im, 0.4509387692675829 + 1.3877787807814457e-17im)

@emmt
Copy link

emmt commented Dec 11, 2023

Compared to the base version, this PR automatically uses the correct numerical precision for all computations because there are no Float64 constants.

Any idea why this has not yet been merged?

@Sagnac
Copy link

Sagnac commented Dec 11, 2023

It's possible @kiranshila just forgot about it. I've been using curio-sitas's fork and it works well.

@kiranshila
Copy link
Owner

Thanks for the ping! Not sure why, but this is the first I've seen this PR, GitHub failed to send me any notifications on any of this, wild!

@curio-sitas this looks fantastic! I'll merge when I get back to my desk.

@kiranshila
Copy link
Owner

I'll update this for all the modern julia pacakging stuff too (haven't thought about this repo since I made it for my Antennas class 4 years ago!) :)

@kiranshila kiranshila merged commit aafe4c8 into kiranshila:master Dec 11, 2023
@kiranshila
Copy link
Owner

I'll also update the readme to include these new benchmarks

@kiranshila
Copy link
Owner

kiranshila commented Dec 11, 2023

@curio-sitas @emmt @Sagnac @devmotion Merged, updated, and pushed to the general registry as v0.2. Thanks for your work, Cheers!

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.

5 participants