Closed schillic closed 2 weeks ago
Sorry I do not know this repository well. Where is the issue with the conversion with Compl
? Typically, for compatibility with 0.21, convert(Interval, a)
should be interval(a)
and, similarly, convert(Interval{T}, a)
should read interval(T, a)
.
About the third item in your list, is it about
? If so, things like interval(T(...), T(...))
should be written interval(T, ..., ...)
.
About the third item in your list, is it about
https://github.com/JuliaIntervals/IntervalRootFinding.jl/blob/754c005c3354faf8a46ef803a2845378b7b4c065/test/slopes.jl#L12-L24 ? If so, things like
interval(T(...), T(...))
should be writteninterval(T, ..., ...)
.
No, that part was fine. I changed it anyway now. And also added some more fixes.
Where is the issue with the conversion with
Compl
? Typically, for compatibility with 0.21,convert(Interval, a)
should beinterval(a)
and, similarly,convert(Interval{T}, a)
should readinterval(T, a)
.
This all happens internally in automatic promotion. Below is an example from a failing test:
Xc = Complex(X, X)
f(z) = z^3 - 1
# Default
rts = roots(f, Xc)
In the call to roots
the function f
is executed with a IntervalRootFinding.Compl{Interval{Float64}},
argument . Then f
says to subtract 1
. So Julia calls the method
-(x::IntervalRootFinding.Compl{Interval{Float64}}, y::Int64)
,
which itself calls the promotion rule.
I tried to fix this by adding another promotion rule, but then got stuck in ForwardDiff
with yet another promotion issue.
Mhmm so since promotion between Number
and interval types are disallowed, the fix may be to define f(z) = z^2 - interval(1)
(since promotion between Compl
and interval
seems to work).
Mhmm so since promotion between
Number
and interval types are disallowed, the fix may be to definef(z) = z^2 - interval(1)
(since promotion betweenCompl
andinterval
seems to work).
To me the whole point of interval arithmetic is that you can plug an interval into a function that expects a number and it will just work. Having to explicitly write interval(1)
means I have to define a new function for that purpose. This is not convenient, and often not even possible if the code lives in another package.
But since this is not my package, I let you decide how to fix things. As I said, feel free to continue from this branch if you want.
Thank you very much for your input on this and taking the time to open this draft 🙂
We do allow the mixing +,*,-(::Interval, ::Number)
, so instead of redefining f
, what could be done is to define the methods +,*,-(::Compl{<:Interval}, ::Number)
...
That being said, since I do not know this package well, I am also not sure what is the exact purpose of Compl
. @lbenet, @Kolaru, would defining a ComplexInterval
type in IntervalArithmetic.jl be better in the long run? In particular, can such a struct
completely replace the need of Compl
?
I tried to upgrade
IntervalArithmetic
to v0.21 but failed with several blockers. I pushed this so somebody with more knowledge can continue. Below are the blocking problems I found.Problems
StaticArrays
matrix multiplication is broken (reported in https://github.com/JuliaIntervals/IntervalArithmetic.jl/issues/573).Compl
uses fallback methods that require anInterval
constructor.setprecision
was removed (it is unclear to me how the behavior is imitated).BigFloat(::Interval{BigFloat})
.