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

Remove extra rounding mode changes for sqrt for interval #8611

Open
wants to merge 1 commit into
base: 5.6.x-branch
Choose a base branch
from

Conversation

sloriot
Copy link
Member

@sloriot sloriot commented Nov 15, 2024

The only place where the function is call is in Interval_nt::sqrt() and that functions start with the creation of a protector.
Even worse, if we are using interval with no protection, we get an incorrect rounding mode after a call to sqrt.

sloriot added a commit to sloriot/cgal that referenced this pull request Nov 15, 2024
Remove extra rounding mode changes for sqrt for interval
@mglisse
Copy link
Member

mglisse commented Nov 15, 2024

Uh? I agree that it isn't optimal (there is already a comment saying so), it may even be wrong in the no-protection case as you mention, but completely removing the only place that sets the rounding mode to "down" cannot be right. For the lower bound, we need to call sqrt with the rounding mode set towards -inf or 0 (all other interval operations use +inf).

@sloriot
Copy link
Member Author

sloriot commented Nov 15, 2024

Uh? I agree that it isn't optimal (there is already a comment saying so), it may even be wrong in the no-protection case as you mention, but completely removing the only place that sets the rounding mode to "down" cannot be right. For the lower bound, we need to call sqrt with the rounding mode set towards -inf or 0 (all other interval operations use +inf).

As stated above, the only function calling it has a protector here so it seems correct to me.

@mglisse
Copy link
Member

mglisse commented Nov 15, 2024

As stated above, the only function calling it has a protector here so it seems correct to me.

That protector sets the rounding mode to UP, which is good for the upper bound, but not for the lower bound, which needs the rounding mode set to DOWN. For other operations, we use tricks (-(-a)+(-b) instead of a+b for instance) so we can emulate rounding DOWN using UP, but for sqrt I am not aware of any such trick, we really need to compute sup and inf using 2 different rounding modes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants