-
Notifications
You must be signed in to change notification settings - Fork 2
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
overestimation free quadratic expansion #175
Conversation
indeed, i didn't spot other places where it is used. otoh, see for example here, method that wants to compute the term IδW = Iδ + 1/2 * δ^2 * A + 1/6 * δ^3 * A² that method could be improved by using IδW = Iδ + quadratic_expansion(A, 1/2 * δ^2, 1/6 * δ^3) |
@lucaferranti how about that we add the tests from #109 (comment) ? |
what about the quadratic_expansion(A::IntervalMatrix{T}, t) where {T} from what I can tell, it's the same of the more general one with quadratic_expansion(A::IntervalMatrix{T}, t) where {T} = quadratic_expansion(A, t, 1/2*t^2) |
julia> t = 1.0
1.0
julia> α = 1.0
1.0
julia> β = 0.5
0.5
julia> A = IntervalMatrix(randn(100, 100) .± abs.(randn(100, 100)));
julia> @btime quadratic_expansion($A, $t);
135.591 ms (7544198 allocations: 115.27 MiB)
julia> @btime quadratic_expansion($A, $α, $β);
71.674 ms (3606 allocations: 296.91 KiB) so maybe we could define quadratic_expansion(A, t) = quadratic_expansion(A, t, t^2/2) |
yes, the code refactoring that you propose sounds good to me |
quick question about the docstring, it currently says
would it be better to keep it "application agnostic" and say that |
True. Maybe it is better to just write |
the function is exported though, is it used in some other packages? According to JuliaHub, it has zero dependents Just trying to avoid breaking things in other packages, although semantic versioning would take care things don't break by accident |
about removing the reason to have a special method is that it's the canonical case (for the matrix exp expansion). |
just double checking one last time before pulling the trigger, remove or replace |
As you prefer. I think removing is good because it simplifies the code base. |
added the test for quadratic expansion from the issue comment, now this should be ready. I noticed the tests for exponentials aren't really testing the result is correct, just that the output is an interval matrix, but I guess that can be addressed in a different PR |
Yes, we were eager to see results back then 😅 |
for the release, since this is breaking and there are at least a couple of other breaking changes coming in the upcoming days (moving exponential to ILA, bringing rump multiplication here, etc.), maybe we can wait till Tuesday meeting for the next release? |
Just for the record, I went back to the method that was removed here and compared it to the new implementation. The old implementation gives slightly more precise bounds on some random example. Not sure if this is general or a coincidence, and why (could be just accumulated rounding errors due to different arithmetic operations). julia> A = rand(IntervalMatrix)
2×2 IntervalMatrix{Float64, IntervalArithmetic.Interval{Float64}, Matrix{IntervalArithmetic.Interval{Float64}}}:
[-1.37152, 0.237901] [-0.502148, 0.325257]
[-0.353947, 0.231257] [1.13864, 1.22323]
julia> B = quadratic_expansion(A, 0.1) # old implementation
2×2 IntervalMatrix{Float64, IntervalArithmetic.Interval{Float64}, Matrix{IntervalArithmetic.Interval{Float64}}}:
[-0.128327, 0.0249617] [-0.0538833, 0.0349019]
[-0.0379805, 0.0248152] [0.119766, 0.130693]
julia> C = quadratic_expansion(A, 0.1, 0.5*0.1^2) # new implementation
2×2 IntervalMatrix{Float64, IntervalArithmetic.Interval{Float64}, Matrix{IntervalArithmetic.Interval{Float64}}}:
[-0.128327, 0.0249617] [-0.0538833, 0.0349019]
[-0.0379805, 0.0248152] [0.119766, 0.130693]
# they are very similar, which needs to be checked in an awkward way because ≈ does not work for Interval
julia> all(B[i, j].lo ≈ C[i, j].lo for (i, j) in [(1,1), (1,2), (2,1), (2,2)])
true
julia> all(B[i, j].hi ≈ C[i, j].hi for (i, j) in [(1,1), (1,2), (2,1), (2,2)])
true
# how are they formally related?
julia> B ⊆ C
true
julia> C ⊆ B
false |
what is the order of magnitude of the overestimation? I'd guess it's due to directed rounding. Are you sure the previous implementation was rigorous? |
for example, taking a patch from the previous implementation a = inf(aii) * t + inf(aii)^2 * t2d2
b = sup(aii) * t + sup(aii)^2 * t2d2
return min(a, b) those are floating point operations subject to rounding error, the returned minimum could be 1e-15 bigger that the true minimum. If you compute |
Yes, that could very well be the explanation. The difference is in that order. You are right, I should not have said "more precise." |
fixes #109
With a quick look at the
exponential.jl
file, that function isn't used anywhere else anyway?