-
Notifications
You must be signed in to change notification settings - Fork 71
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
overload isapprox for intervals #480
Conversation
Codecov Report
@@ Coverage Diff @@
## master #480 +/- ##
==========================================
- Coverage 91.56% 91.55% -0.01%
==========================================
Files 25 25
Lines 1755 1753 -2
==========================================
- Hits 1607 1605 -2
Misses 148 148
Continue to review full report at Codecov.
|
This is one of the case where we need to be very careful. There are two possible strategy (already discussed several times in e.g. #181): (a) We do the most straightforward thing, so for this case two intervals are approximately equals if their respective bounds are. This is what is implemented here. However it may break code written for numbers in unexpected ways. This is also related to https://github.com/gwater/NumberIntervals.jl as an actual implementation of (b) and #271 that tried to build the ground work to solve all these issues. Unfortunately I don't know when I will have the time/energy to go back to the latter. |
Hups, apologies for rushing into the PR without the proper background check, I was aware of the issues/PRs you linked but when I opened this I did not connect the dots. I can close this PR for the time being, but I do have a couple of comments:
This is happening with the current version, which is falling back to Regardless of what the correct behavior of isapprox should be, the current one is not, julia> a = 1..(1+1e-16)
[1, 1]
julia> b = a + 1e-308
[1, 1.00001]
julia> a ≈ b
false
julia> a.lo ≈ b.hi
true I chose this example because in this case any number in a is approximately equal to any number in b (with the default tolerance settings), so this should be true in both philosophies. Always returning Moreover, I would say that generic code written for |
Also, reading at the old related issues, my main take-home understanding (but I wouldn't be surprised if I misunderstood something) is that:
I agree with this! Technically speaking, floating-point numbers do not obey the axioms of real numbers either, since e.g. Hence I agree that having However, what about having I am sure you had already thought about this and I apologize if it feels like we are having just another instance of an old conversation. I will re-read the discussions in the related issues and PRs to make sure I don't waste anyone's time by reinventing the wheel. For what is worth, I do think that repeating an old conversation after some time is not necessarily a waste of time, as it can bring up new insights 😄 |
First no need to apologize, I pointed out old issues because I thought you may find them interesting, not because I expected you to know about them :) (this goes as far back as #2 so it is quite a rabbit hole)
I think that underlines the core of the problem. You are right, always returning
An example would be a possible Taylor expansion of a function if a ≈ b
# Do a Taylor expansion of the function around x
return f(0) + (a - b) f'(0)
else
# Go the expensive route
return f(a - b)
end If you input the interval in you example It relies of one of the properties of numbers that intervals are missing ( That kind of branching is not unusual for the computation of special functions like the zeta function (however it generally uses inequalities I think).
A lot of packages require Another course of action (briefly discussed on slack) would be to encourage removal of this type restriction everywhere and mostly rely on duck typing. But that would be a more global discussion. This is however tangential to the problem of boolean comparison of intervals. |
thank you for the example, it was an interesting one, also apologies for returning to this so seldomly. I promise that next time I'll answer faster 😄 The current implementation of function ==(a::Interval, b::Interval)
isempty(a) && isempty(b) && return true
a.lo == b.lo && a.hi == b.hi
end which is in accordance to philosophy a) of your original message. This may change in the future and if in the future we will have the flavour based system with different mechanisms, but since at the moment the "set flavor" is being used for equality, I don't understand why not use the same for approximate inequality, that is here in the PR I am mainly after coherence in the package. The current fall back implementation is failing because of norm(x-y) <= max(atol, rtol*max(norm(x), norm(y)))) and |
Also, regarding packages interoperability, preventing it is not always a bad thing. For example, plugging interval matrices into traditional linear algebra algorithms is usually a bad idea, let's take an example julia> using IntervalArithmetic, LinearAlgebra
julia> A = [1..2 1..2;2..2 3..3]
2×2 Matrix{Interval{Float64}}:
[1, 2] [1, 2]
[2, 2] [3, 3]
julia> lu(A)
LU{Interval{Float64}, Matrix{Interval{Float64}}}
L factor:
2×2 Matrix{Interval{Float64}}:
[1, 1] [0, 0]
[1, 2] [1, 1]
U factor:
2×2 Matrix{Interval{Float64}}:
[1, 2] [1, 2]
[0, 0] [-1, 2] everything seems nice we love having packages just magically work. But let us take now a nastier example julia> A = [1e-16..2 1..2;2..2 3..3]
2×2 Matrix{Interval{Float64}}:
[1e-16, 2] [1, 2]
[2, 2] [3, 3]
julia> lu(A)
LU{Interval{Float64}, Matrix{Interval{Float64}}}
L factor:
2×2 Matrix{Interval{Float64}}:
[1, 1] [0, 0]
[1, 2e+16] [1, 1]
U factor:
2×2 Matrix{Interval{Float64}}:
[1e-16, 2] [1, 2]
[0, 0] [-4e+16, 2]
This and the fact that |
@lucaferranti what is the status of this PR? Perhaps rebase and define some |
I am closing this PR since it seems abandoned. Feel free to re-open it if you want to revive it at some point, or just open a new one. |
isapprox
wasn't overloaded for intervals and was using the fallbackisapprox(a::Number, b::Number)
. This PR defines the methodisapprox(a::Interval, b::Interval)
before
after
Related issues:
fixes #479