Skip to content
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

Example 5.5 from Smiley and Chun (2001) solved inconsistently #86

Closed
gwater opened this issue Jun 27, 2018 · 20 comments
Closed

Example 5.5 from Smiley and Chun (2001) solved inconsistently #86

gwater opened this issue Jun 27, 2018 · 20 comments

Comments

@gwater
Copy link
Contributor

gwater commented Jun 27, 2018

The example in test/smiley_examples.jl has 41 roots, however, only 20 are found on some setups. On other setups all 41 are identified correctly. This inconsistency was first discovered and discussed in #70.

@gwater
Copy link
Contributor Author

gwater commented Jun 27, 2018

Some progress on triangulating the issue: On my machine the example works with v0.7.0 of StaticArrays.jl and breaks with v0.7.2.

@gwater
Copy link
Contributor Author

gwater commented Jun 27, 2018

The example is already broken with v0.7.1 of StaticArrays.jl

@gwater
Copy link
Contributor Author

gwater commented Jun 27, 2018

git bisection indicates that the problem is related to this commit in StaticArrays.jl

@dpsanders
Copy link
Member

Great detective work, thanks. Would you like to report it at StaticArrays.jl?

@gwater
Copy link
Contributor Author

gwater commented Jun 27, 2018

Sure, it cannot hurt to have them take a look at it. The code in question is called here, correct?

@dpsanders
Copy link
Member

Yes, that's the right place.

@gwater
Copy link
Contributor Author

gwater commented Jun 29, 2018

Given that the next version of StaticArrays will most likely contain a fix for this issue, I think v0.7.1 through v0.7.2 of StaticArrays.jl should be blacklisted in IntervalRootFinding.jl's REQUIRE to protect users from incorrect results.

@gwater
Copy link
Contributor Author

gwater commented Jun 29, 2018

The discussion in the StaticArrays issue raised questions about the definition of isinf(::Interval). Looking at its current definition I'm concerned, too:

isentire(x::Interval) = x.lo == -Inf && x.hi == Inf
isinf(x::Interval) = isentire(x)

This definition seems both geneerous and too specific at the same time. My understanding is that the question "Is this number infinite?" cannot be answered at all for an interval that contains both an infinity and finite numbers. It could be both finite or infinite, and we don't know yet.

Should I open an issue to discuss this further?

@dpsanders
Copy link
Member

Yes, I agree. Please do open an issue, yes.

@dpsanders
Copy link
Member

I think the solution for us is to use the work in #67.
cc @eeshan9815

lbenet referenced this issue in JuliaIntervals/IntervalArithmetic.jl Jun 29, 2018
* Add methods for IntervalBox previously in IntervalRootFinding

* Remove a reference to IntervalRootFinding, and fix complex interval test

* Implement suggestions of the review

* Incorporate a missing suggestion of the review
@gwater
Copy link
Contributor Author

gwater commented Jun 29, 2018

Yes, it makes sense to have a safe implementation of left divide within the package since that operation is so crucial to the Newton method.

@eeshan9815
Copy link
Contributor

So should the methods in #67 be used instead of Base.\ by importing and modifying it?

@dpsanders
Copy link
Member

Yes, that's probably a good solution. Let's try it.

@dpsanders
Copy link
Member

Maybe make it do Gaussian elimination for now.
But probably there should be a way to choose in the roots function which type of linear solution to use.

@eeshan9815
Copy link
Contributor

Should it use the preconditioned version or the normal one?

@dpsanders
Copy link
Member

There should be an option to either use it or not, and test both to see which is better!

@dpsanders
Copy link
Member

I think this will be solved by using interval Gauss-Seidel instead of Gaussian elimination to solve the linear system. @Kolaru is it possible to specify this option?

@Kolaru
Copy link
Collaborator

Kolaru commented Mar 24, 2019

@dpsanders Currently it's not possible, the contractors using the \ operator (and thus whatever it is bind to).

I've added it to the list in #116 of what should be possible to pass to roots.

@gwater
Copy link
Contributor Author

gwater commented Aug 1, 2023

Silent failure should be solved by JuliaIntervals/IntervalArithmetic.jl#571

@gwater
Copy link
Contributor Author

gwater commented Aug 11, 2023

With the earlier change to the contractors (using a custom \ operation) and the restriction on boolean operations in IntervalArithmetic.jl this issue can be considered resolved. Both precautions together should be sufficient.

@gwater gwater closed this as completed Aug 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants