-
Notifications
You must be signed in to change notification settings - Fork 9
Support Jacobi(-1,0)\Ultraspherical(-1/2) and Jacobi(0,-1) #245
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
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #245 +/- ##
==========================================
+ Coverage 90.99% 91.03% +0.03%
==========================================
Files 19 19
Lines 1744 1751 +7
==========================================
+ Hits 1587 1594 +7
Misses 157 157 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Also added Jacobi(0,-1) |
function \(A::Jacobi, B::Ultraspherical) | ||
B̃ = Jacobi(B) | ||
(A\B̃)*Diagonal(B[1,:]./B̃[1,:]) | ||
if B == Ultraspherical(-1/2) && (A == Jacobi(-1, 0) || A == Jacobi(0, -1)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add a comment why this special case is needed.
Also, where in (A\B̃)*Diagonal(B[1,:]./B̃[1,:])
are the NaN
s coming in?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The NaN
s come from B̃[1,:]
, since
julia> Jacobi(-1, -1)[1, :]
ℵ₀-element view(::Jacobi{Float64, Int64}, 1.0, :) with eltype Float64 with indices OneToInf():
1.0
0.0
NaN
NaN
NaN
NaN
NaN
NaN
NaN
NaN
⋮
(since we haven't yet given a definition to Jacobi(-1, -1)
in the code that works, although it'd be all zero anyway -except for the first two entries- I think?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Won't it still lead to a NaN
anyway since
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Touché
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
update the comment to mention even if it is defined the above won't work because of division by zero.
On the current version,
This adds a fix, noting that