-
Notifications
You must be signed in to change notification settings - Fork 3
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
Implement b=-1 #111
Implement b=-1 #111
Conversation
I'm looking at this now, will try to get the tests to pass |
Will hopefully have this all fixed up soon after I get out of a whole rabbit hole of ambiguities. The expansion tests started failing after #112 for some reason |
What's the error? Or does it just give wrong values in the expansion without erroring? If it's a problem in that PR let me know. |
I think it's just issues with the layouts. Not really an issue with the pr,
probably just that it's revealed some other issues with ambiguities. I'll
give an example of the error in the morning
…On Tue, 2 July 2024, 1:02 am Timon Salar Gutleb, ***@***.***> wrote:
What's the error? Or does it just give wrong values in the expansion
without erroring? If it's a problem in that PR let me know.
—
Reply to this email directly, view it on GitHub
<#111 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AWZPH4G3LWTVBYUA3RW6JOTZKHUZ5AVCNFSM6AAAAABJSNRMSOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDEMBRGQ3TGOJXHA>
.
You are receiving this because you authored the thread.Message ID:
<JuliaApproximation/SemiclassicalOrthogonalPolynomials.
***@***.***>
|
@TSGut This is the issue: julia> using SemiclassicalOrthogonalPolynomials, ClassicalOrthogonalPolynomials # make sure you're on this pr branch
julia> Ps = SemiclassicalJacobi.(2, -1//2:5//2, -1.0, -1//2:5//2)
4-element SemiclassicalOrthogonalPolynomials.SemiclassicalJacobiFamily{Float64, UnitRange{Rational{Int64}}, Float64, UnitRange{Rational{Int64}}}:
SemiclassicalJacobi with weight x^-0.5 * (1-x)^-1.0 * (2.0-x)^-0.5 on 0..1
SemiclassicalJacobi with weight x^0.5 * (1-x)^-1.0 * (2.0-x)^0.5 on 0..1
SemiclassicalJacobi with weight x^1.5 * (1-x)^-1.0 * (2.0-x)^1.5 on 0..1
SemiclassicalJacobi with weight x^2.5 * (1-x)^-1.0 * (2.0-x)^2.5 on 0..1
julia> g = x -> exp(x) + sin(x)
#11 (generic function with 1 method)
julia> expand(Ps[1], g) # works
SemiclassicalJacobi with weight x^-0.5 * (1-x)^-1.0 * (2.0-x)^-0.5 on 0..1 * [3.559752813266927, -2.689556624759561, -0.1419489233276318, -0.00815710939820469, -0.0012292671637252207, -7.007691099937732e-5, -1.3644779541294862e-6, -3.3513086473469194e-8, -2.7275540904437935e-9, -8.832973058841338e-11 … ]
julia> expand(Ps[2], g) # works
SemiclassicalJacobi with weight x^0.5 * (1-x)^-1.0 * (2.0-x)^0.5 on 0..1 * [3.559752813266927, -2.813114026394983, -0.15330885488899992, -0.009528747519073118, -0.0013411342705156972, -7.427293498329727e-5, -1.4527735961828238e-6, -3.734131077723306e-8, -2.925216076248728e-9, -9.315982251717606e-11 … ]
julia> expand(Ps[3], g)
ERROR: MethodError: simplifiable(::ArrayLayouts.Mul{LazyArrays.ApplyLayout{typeof(\)}, ArrayLayouts.DiagonalLayout{ArrayLayouts.OnesLayout}, LazyArrays.ApplyMatrix{Float64, typeof(\), Tuple{LinearAlgebra.UpperTriangular{Float64, InfiniteLinearAlgebra.AdaptiveQRFactors{Float64, BandedMatrices.BandedMatrix{Float64, Matrix{Float64}, Base.OneTo{Int64}}, LazyBandedMatrices.SymTridiagonal{Float64, LazyArrays.BroadcastVector{Float64, typeof(-), Tuple{Float64, ClassicalOrthogonalPolynomials.LanczosJacobiBand{Float64}}}, LazyArrays.BroadcastVector{Float64, typeof(-), Tuple{ClassicalOrthogonalPolynomials.LanczosJacobiBand{Float64}}}}}}, LinearAlgebra.Diagonal{Float64, LazyArrays.BroadcastVector{Float64, typeof(*), Tuple{LazyArrays.BroadcastVector{Float64, typeof(sign), Tuple{SubArray{Float64, 1, LinearAlgebra.UpperTriangular{Float64, InfiniteLinearAlgebra.AdaptiveQRFactors{Float64, BandedMatrices.BandedMatrix{Float64, Matrix{Float64}, Base.OneTo{Int64}}, LazyBandedMatrices.SymTridiagonal{Float64, LazyArrays.BroadcastVector{Float64, typeof(-), Tuple{Float64, ClassicalOrthogonalPolynomials.LanczosJacobiBand{Float64}}}, LazyArrays.BroadcastVector{Float64, typeof(-), Tuple{ClassicalOrthogonalPolynomials.LanczosJacobiBand{Float64}}}}}}, Tuple{InfiniteLinearAlgebra.InfBandCartesianIndices}, false}}}, Float64}}}}}, LinearAlgebra.Diagonal{Float64, FillArrays.Ones{Float64, 1, Tuple{InfiniteArrays.OneToInf{Int64}}}}}) is ambiguous.
Candidates:
simplifiable(M::ArrayLayouts.Mul{LazyArrays.ApplyLayout{typeof(\)}})
@ LazyArrays C:\Users\djv23\.julia\packages\LazyArrays\QwVhu\src\linalg\inv.jl:140
simplifiable(M::ArrayLayouts.Mul{<:Any, <:ArrayLayouts.DiagonalLayout{<:ArrayLayouts.AbstractFillLayout}})
@ LazyArrays C:\Users\djv23\.julia\packages\LazyArrays\QwVhu\src\linalg\mul.jl:375
Possible fix, define
simplifiable(::ArrayLayouts.Mul{LazyArrays.ApplyLayout{…}, <:ArrayLayouts.DiagonalLayout{…}})
Stacktrace:
[1] simplifiable
@ C:\Users\djv23\.julia\packages\LazyArrays\QwVhu\src\linalg\mul.jl:339 [inlined]
[2] _most_simplifiable
@ C:\Users\djv23\.julia\packages\LazyArrays\QwVhu\src\linalg\mul.jl:342 [inlined]
[3] simplifiable(::typeof(*), ::LazyArrays.ApplyMatrix{…}, ::LazyArrays.ApplyMatrix{…}, ::LinearAlgebra.Diagonal{…})
@ LazyArrays C:\Users\djv23\.julia\packages\LazyArrays\QwVhu\src\linalg\mul.jl:340
[4] _simplify(::typeof(*), ::LazyArrays.ApplyMatrix{…}, ::LazyArrays.ApplyMatrix{…}, ::LinearAlgebra.Diagonal{…}, ::Vararg{…})
@ LazyArrays C:\Users\djv23\.julia\packages\LazyArrays\QwVhu\src\linalg\mul.jl:349
[5] simplify
@ C:\Users\djv23\.julia\packages\LazyArrays\QwVhu\src\linalg\mul.jl:345 [inlined]
[6] simplify
@ C:\Users\djv23\.julia\packages\LazyArrays\QwVhu\src\linalg\mul.jl:355 [inlined]
[7] copy
@ C:\Users\djv23\.julia\packages\LazyArrays\QwVhu\ext\LazyArraysBandedMatricesExt.jl:576 [inlined]
[8] materialize
@ C:\Users\djv23\.julia\packages\ArrayLayouts\70nvu\src\mul.jl:137 [inlined]
[9] mul
@ C:\Users\djv23\.julia\packages\ArrayLayouts\70nvu\src\mul.jl:138 [inlined]
[10] *(A::LazyArrays.ApplyArray{…}, B::LazyBandedMatrices.Bidiagonal{…})
@ ArrayLayouts C:\Users\djv23\.julia\packages\ArrayLayouts\70nvu\src\mul.jl:223
[11] \(A::SemiclassicalJacobi{Float64}, B::SemiclassicalJacobi{Float64})
@ SemiclassicalOrthogonalPolynomials c:\Users\djv23\.julia\dev\SemiclassicalOrthogonalPolynomials.jl\src\SemiclassicalOrthogonalPolynomials.jl:446
[12] \(A::SemiclassicalJacobi{Float64}, B::SemiclassicalJacobi{Float64})
@ SemiclassicalOrthogonalPolynomials c:\Users\djv23\.julia\dev\SemiclassicalOrthogonalPolynomials.jl\src\SemiclassicalOrthogonalPolynomials.jl:435
[13] semijacobi_ldiv(P::SemiclassicalJacobi{…}, Q::LanczosPolynomial{…})
@ SemiclassicalOrthogonalPolynomials c:\Users\djv23\.julia\dev\SemiclassicalOrthogonalPolynomials.jl\src\SemiclassicalOrthogonalPolynomials.jl:278
[14] \(A::SemiclassicalJacobi{Float64}, B::LanczosPolynomial{Float64, SemiclassicalJacobiWeight{…}, Normalized{…}})
@ SemiclassicalOrthogonalPolynomials c:\Users\djv23\.julia\dev\SemiclassicalOrthogonalPolynomials.jl\src\SemiclassicalOrthogonalPolynomials.jl:452
[15] ldiv(Q::SemiclassicalJacobi{Float64}, f::QuasiArrays.BroadcastQuasiVector{Float64, var"#11#12", Tuple{Inclusion{…}}})
@ SemiclassicalOrthogonalPolynomials c:\Users\djv23\.julia\dev\SemiclassicalOrthogonalPolynomials.jl\src\SemiclassicalOrthogonalPolynomials.jl:519
[16] \(A::SemiclassicalJacobi{Float64}, B::QuasiArrays.BroadcastQuasiVector{Float64, var"#11#12", Tuple{Inclusion{…}}})
@ QuasiArrays C:\Users\djv23\.julia\packages\QuasiArrays\Jo8rC\src\matmul.jl:34
[17] transform(A::SemiclassicalJacobi{Float64}, f::Function)
@ ContinuumArrays C:\Users\djv23\.julia\packages\ContinuumArrays\d6Ead\src\bases\bases.jl:291
[18] expand(A::SemiclassicalJacobi{Float64}, f::Function)
@ ContinuumArrays C:\Users\djv23\.julia\packages\ContinuumArrays\d6Ead\src\bases\bases.jl:302
[19] top-level scope
@ c:\Users\djv23\.julia\dev\SemiclassicalOrthogonalPolynomials.jl\test\test_neg1b.jl:80
Some type information was truncated. Use `show(err)` to see complete types. Part of the issue is probably what I've had to do here to fix a new |
So replacing the line The long term fix would be to disambiguate in LazyArrays.jl, the error message is pretty spot on, the issue is these two options are ambiguous for our multiplication chain of Candidates:
simplifiable(M::ArrayLayouts.Mul{LazyArrays.ApplyLayout{typeof(\)}})
@ LazyArrays C:\Users\djv23\.julia\packages\LazyArrays\QwVhu\src\linalg\inv.jl:140
simplifiable(M::ArrayLayouts.Mul{<:Any, <:ArrayLayouts.DiagonalLayout{<:ArrayLayouts.AbstractFillLayout}})
@ LazyArrays C:\Users\djv23\.julia\packages\LazyArrays\QwVhu\src\linalg\mul.jl:375 A well placed redirection if one is in both of those cases would be one example of a proper fix. In any case that has to happen in LazyArrays.jl. For now I would recommend the quick fix I mentioned. |
Thanks for looking into it @TSGut. I am still getting used to understanding all of the design across these packages :) I'll use this temporary fix for now but I'll try and get some fixes through as well. |
I would suggest in general being explicit about Lazy array constructions as it will reduce compile time |
Any ideas why the first testset (the Jacobi test in the test_neg1b.jl file) might have become insanely slow all of a sudden? I don't believe I did anything significant.. the slowness is coming only from the first evaluation for each |
Hmm its possible the changes in LazyArrays.jl I just made have made compilation really slow... |
It might be Windows specific? I'm having trouble with these tests passing in time: |
I started seeing this issue last night but only got to confirm it just now, so I'm not sure it'd be those changes just now. If it's Windows only that'll be pretty rough to debug |
Can you pin LazyArrays.jl at 2.1.4 and see if it goes away? If it is a compile time issue there might be a type instability introduced |
Also, are you on LazyArrays v2.1.5 or 2.1.6? If 2.1.5 can you try 2.1.6 just in case the issue is gone? |
I was on v2.1.5. The issue is still on 2.1.6. It's gone on 2.1.4 |
Would it somehow be due to JuliaArrays/LazyArrays.jl#334? This is the only method that I think might be encountered that changed in 2.1.5 as far as I can tell (not 100% sure what JuliaArrays/LazyArrays.jl#335 does). |
Can you try reverting just that commit and see if it fixes the issue? |
Reverting JuliaArrays/LazyArrays.jl#334 didn't change anything. I'll have to learn how to properly revert JuliaArrays/LazyArrays.jl#335 and I'll test |
Hopefully I did the revert properly here DanielVandH/LazyArrays.jl@6fd2bc8. This revert does fix the issues I'm seeing. |
I suspect the answer is the former is inlining and removing all the So I think for now we just make the fix you found. The design of |
Been spending ages again trying to figure out why my function colsupport(lay::AbstractInvLayout{<:TriangularLayout}, A, j)
B, = arguments(lay, A)
return colsupport(B, j)
end
function rowsupport(lay::AbstractInvLayout{<:TriangularLayout}, A, k)
B, = arguments(lay, A)
return rowsupport(B, k)
end I added makes them pass again. Is there something wrong with these definitions? Or any suggestions about what could be happening? I'm going to make a PR to revert them for the time being but it would be nice to try and see what the issue is. Actually it seems to be a combination of this and something else I need to identify.. I'll keep trying. |
Ignore the above. It has nothing to do with those methods. Something else between [email protected] and [email protected] is breaking my tests, but it's not that. I don't know what it is yet though.. none of the other tests fail, its only my |
There were a few other similar changes I made replacing AbstractPaddedLayout, try changing those back? |
I don't know what went wrong where I determined that I was wrong initially. Turns out that reverting it so fix my problems. I'm going to make a PR reverting it, but I don't fully understand the issue. It would be nice to eventually get that method back in. |
Is it a compile time thing or run time? Try throwing an error? |
It's runtime. The coefficients from |
…e parameters take that long) to ~1m
Do we need derivatives of fully weighted CI will also fail until JuliaLinearAlgebra/InfiniteLinearAlgebra.jl#186 gets merged and registered. Might also want JuliaArrays/LazyArrays.jl#341 done before completing this |
Everything should be good now. Tests all pass locally. Not sure what CI will do until
is resolved. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #111 +/- ##
==========================================
+ Coverage 92.52% 94.32% +1.80%
==========================================
Files 3 3
Lines 575 634 +59
==========================================
+ Hits 532 598 +66
+ Misses 43 36 -7 ☔ View full report in Codecov by Sentry. |
Need to get #109, #105, and JuliaArrays/LazyArrays.jl#320 (or something like it) over the line to get this completed. There could be a way to just force it all to work by putting
ApplyArray
s everywhere (though there are issues with those too, I've found) but getting those done first will probably be more useful in the long run. I'll mark this as ready to review once that's done. Tests did pass when I last worked on this four days ago, not sure where all these ambiguities have come from but I'll work on it eventually.This PR implements$P_n^{t, (u, -1, w)}$ , defined so that
with$P_0^{t, (u, -1, w)}(y) = 1$ . The Jacobi matrix with this definition is defined by
with
Differentiation is given by
where
and
Need to add weighted derivatives