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

Refactor code into separate modules #84

Closed
wants to merge 15 commits into from

Conversation

heltonmc
Copy link
Member

@heltonmc heltonmc commented Mar 17, 2023

As the project grows it might be best to separate namespaces into different modules. The motivation is that the constant polynomials take up a lot of namespace and it might be best to have unique functions into different modules.

In the future if someone wanted to split the package into smaller sub-packages this will help make that transition easier as well as be helpful as I add other functions (Scorer, Struve).

As of now I'm most interested how this affects package load / compile times so I will need to test that and clean some things up.

I think the pathway right now is:

  1. get Use explicit SIMD instructions for parallel evaluation of polynomials #79 merged
  2. merge this PR,
  3. use the SIMD methods for the real Airy functions
  4. Reorganize the complex airy functions for better SIMD (also not depend on Bessel function computation which is slow)
  5. Use the better airy functions in uniform airy type expansions for the Bessel function code
  6. Use those to have routines for complex support in bessel functions

Sounds like a lot but mostly there......

@codecov
Copy link

codecov bot commented Mar 17, 2023

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (16ddeb6) 96.54% compared to head (52001b5) 96.55%.

Additional details and impacted files
@@           Coverage Diff           @@
##           master      #84   +/-   ##
=======================================
  Coverage   96.54%   96.55%           
=======================================
  Files          20       23    +3     
  Lines        2373     2377    +4     
=======================================
+ Hits         2291     2295    +4     
  Misses         82       82           
Impacted Files Coverage Δ
src/AiryFunctions/airy.jl 99.56% <ø> (ø)
src/AiryFunctions/cairy.jl 85.06% <ø> (ø)
src/BesselFunctions/Float128/besselj.jl 98.07% <ø> (ø)
src/BesselFunctions/Float128/bessely.jl 96.07% <ø> (ø)
src/BesselFunctions/Polynomials/besselj_polys.jl 100.00% <ø> (ø)
src/BesselFunctions/U_polynomials.jl 99.28% <ø> (ø)
src/BesselFunctions/asymptotics.jl 99.48% <ø> (ø)
src/BesselFunctions/besseli.jl 99.00% <ø> (ø)
src/BesselFunctions/besselj.jl 98.42% <ø> (ø)
src/BesselFunctions/besselk.jl 97.27% <ø> (ø)
... and 13 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@oscardssmith
Copy link
Member

I really feel like the SIMD polynomials should probably be a separate package since

  1. Other people might want to use them (but not care about bessel functions)
  2. Separation of concerns: If someone makes the polynomial code faster, the bessel code doesn't have to change.

@heltonmc
Copy link
Member Author

Sounds good to me! I've moved the SIMD code over to https://github.com/heltonmc/SIMDMath.jl and will register later and come back to this in a couple days.

Are we still ok with keeping the module structure here just minus the SIMDMath code?

@heltonmc
Copy link
Member Author

Registration at JuliaRegistries/General#79804. I'll come back Monday and update the airy functions with that dependency.

I'm starting to wonder now if we just want to separate out all the functions into different modules (AiryFunctions, GammaFunctions, BesselFunctions, ..etc) and then just reexport them here. I know that was briefly discussed in SpecialFunctions.jl but I would definitely like to have some top line package that contains just julia code that brings together all of the special function libraries.

Not for sure if all these different packages would be under JuliaMath or a different org (I don't want to spam too much the CI there).

Right now this reorg exports gamma but open to ideas. This is a similar discussion to #27 where I have gone back and forth....

@heltonmc
Copy link
Member Author

Closed in favor of #95

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