-
Notifications
You must be signed in to change notification settings - Fork 53
Upcoming lu
issue on nightly
#281
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
Comments
Hi @dkarrasch ! Thanks for reporting the future issue. If you can, we would be grateful if you can help us with this. |
Hi, I'm happy to chime in. From the surrounding comments in the code, it seems the goal is to avoid pivoting, which LinearAlgebra would otherwise do by default but it may fail in this case. If that is so, might the following be sufficient? lu(A::AbstractMatrix{Taylor1{T}}; check::Bool = true) where {T<:Number} = lu(A, Val(false); bool=bool) This is not quite the same as the current implementation though, because the code tries again with pivoting if the first attempt fails. So perhaps function lu(A::AbstractMatrix{Taylor1{T}}; check::Bool = true) where {T<:Number}
F = lu(A, Val(false); bool=bool)
if issuccess(F)
return F
else
lu(A, Val(true); bool=bool)
end
end The advantage of the above is that you avoid using LinearAlgebra internals, but perhaps I'm missing something. If it turns out that LinearAlgebra doesn't properly copy the array, say, then we'll have to look into that. |
I tested a little bit and found that the issue is the new |
Thanks @daanhb for chiming in! I guess by now you have already found that the changes implemented in #223 were motivated by a somewhat similar report #221. I'm not an expert in Linear Algebra, so any help from you is welcome. As a side question, do you know if we can test now 1.7 and 1.8 using github actions? |
For 1.7, you would need to "hardcode" it as "1.7". Version 1.8 is available as "nightly", just "1" corresponds to v1.6.1 currently. |
I just tested (locally) Julia v1.7.0-beta2 and got the following warning: Tests for Taylor1 expansions | 375 375
┌ Warning: `lu!(A::Union{StridedMatrix, HermOrSym, Tridiagonal}, ::Val{false}; check::Bool = true)` is deprecated, use `lu!(A, NoPivot(); check = check)` instead.
│ caller = ip:0x0
└ @ Core :-1 Aside from this warning, tests pass fine. |
Yes, the "problematic" PR is not included in v1.7, it's going to be in v1.8, which is why tests will fail now on nightly. But JuliaLang/julia#41288 is going to fix that issue. There is WIP for the deprecation warning, PR is coming soon. |
On current master, there is a finer distinction in
copy_oftype
-like constructors, which will likely make this package fail on nightly, seeJuliaLang/julia#40831 (comment)
This affects your own
lu
implementation, which usescopy_oftype(A, S)
, which used to callconvert(AbstractArray{S}(A))
, but now relies on 2-argsimilar
. So there are two options to fix this:similar
function for matrices with eltypeTaylor1
, thencopy_oftype
will start using this starting from Julia v1.8 "silently".copy_oftype(A, S)
inlu
by a direct call toconvert(AbstractArray{S}, A)
, which is handled internally in your package.If needed, I'd be willing to help, probably just like @daanhb.
The text was updated successfully, but these errors were encountered: