SphericalHarmonicExpansions.jl icon indicating copy to clipboard operation
SphericalHarmonicExpansions.jl copied to clipboard

Published 20 hours ago • verified publisher icon hofmannmartin

Refactoring package

Open fgerick opened this issue 4 years ago • 3 comments

Hi Martin,

I have worked with a previous version of your package and extended it a bit (different norms possible, arbitrary datatypes) during my PhD thesis. It's now in a separate repository: CartesianSphericalHarmonics.jl, as I had no need for the expansion and evaluation of the polynomials. It's not documented nor tested properly. I'd be happy to make this eventually merge with your package or to make the two packages built on each other, any thoughts on this?

Cheers, Felix

fgerick avatar Nov 12 '20 10:11 fgerick

Hi Felix,

it does make sense to include your extensions into our package. Maybe with two small modifications 😉

  1. I think it makes sense to check for integer overflow. Maybe using https://github.com/JeffreySarnoff/SaferIntegers.jl
  2. Maybe change the signature, such that you can call ylmcoeff(Schmidt{Int64},1,1) instead of ylmcoeff(Schmidt{Int64}(),1,1).

Thank you for your contribution, Martin

hofmannmartin avatar Nov 12 '20 13:11 hofmannmartin

I agree with the integer overflow check! For point 2): Can you point me to an implementation in a commonly used Julia code that does this? I'm not aware of this and if it is good/common practice.

fgerick avatar Nov 17 '20 08:11 fgerick

A good example is the convert function in julia base.

julia> x = 1/3
0.3333333333333333

julia> convert(Float32, x)
0.33333334f0

hofmannmartin avatar Nov 17 '20 09:11 hofmannmartin