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

Value and Indexing-by-rational errors #13

Open
kaltsoplyn opened this issue Jan 19, 2023 · 2 comments
Open

Value and Indexing-by-rational errors #13

kaltsoplyn opened this issue Jan 19, 2023 · 2 comments

Comments

@kaltsoplyn
Copy link

Hello, thank you for making this available.
I use it for large quantum numbers and it's super fast. However, I noticed this today:

julia> WignerFamilies.wigner3j_f(Float64, 1, 1//2, 1, -1//2)[3//2]
-0.2886751345948129  # ok

This is ok.

julia>  WignerFamilies.wigner3j_f(Float64, 3//2, 1, -1//2, 1)[1//2]
0.32025630761017426  # not ok

This doesn't look good. It's a cyclic permutation of the previous one, but yields a different value.
And then the next cyclic permutation yields an error due to indexing with a rational:

julia> WignerFamilies.wigner3j_f(Float64, 1//2, 3//2, -1//2, -1//2)[1]
ERROR: ArgumentError: invalid index: 1//1 of type Rational{Int64}
Stacktrace:
 [1] to_index(i::Rational{Int64})
   @ Base .\indices.jl:300
 [2] to_index(A::WignerSymbolVector{Float64, Rational{Int64}, Vector{Float64}}, i::Rational{Int64})
   @ Base .\indices.jl:277   ... etc.

I'm not somehow blundering spectacularly here, am I?
I haven't tested extensively (i.e., I started writing this issue report 2 minutes after I noticed it), but the indexing error seems to occur when all arguments are half-integers. The value error, I noticed it as I was testing the indexing error! I'll take a look at the source code if I find some time later. Cheers. And thanks again,

@xzackli
Copy link
Owner

xzackli commented Jan 20, 2023

Thanks @kaltsoplyn, I imagine this is a bug. I'll look into this!

I am a cosmologist and I wrote this package mostly for spin-0 and spin-2 fields on the sphere, which do not typically involve half-integer indices.

@xzackli
Copy link
Owner

xzackli commented Jan 20, 2023

In the meantime, I believe the BigInt factorization strategy in https://github.com/Jutho/WignerSymbols.jl will be slow but correct.

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

No branches or pull requests

2 participants