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

Implement cancel_minus and cancel_plus #76

Merged
merged 4 commits into from
Feb 27, 2022
Merged

Conversation

Chris00
Copy link
Contributor

@Chris00 Chris00 commented Feb 18, 2022

Implement cancel_minus and cancel_plus. It passes the tests (at least on x86_64). Maybe there is a better implementation but at least it is a basis for discussion.

Depends on unageek/ITF1788#1

@coveralls
Copy link

coveralls commented Feb 18, 2022

Pull Request Test Coverage Report for Build 1906105179

  • 73 of 78 (93.59%) changed or added relevant lines in 2 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.1%) to 83.666%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/basic.rs 61 66 92.42%
Totals Coverage Status
Change from base Build 1896450928: 0.1%
Covered Lines: 2102
Relevant Lines: 2145

💛 - Coveralls

@Chris00 Chris00 force-pushed the cancel branch 7 times, most recently from 62b4ce1 to 4023bf5 Compare February 19, 2022 17:12
@Chris00
Copy link
Contributor Author

Chris00 commented Feb 20, 2022

BTW, given your understanding of the IEEE standard, what should the value of const_interval!(f64::MAX, f64::MAX).cancel_minus(const_interval!(f64::MIN, f64::MIN)) be? Given that the spec says it should be the 𝕋-hull of the level-1 result, IMHO it should be const_interval!(f64::MAX, f86::INFINITY). This is not tested by the suite.

@Chris00
Copy link
Contributor Author

Chris00 commented Feb 20, 2022

(I'm asking in part because IntervalArithmetic.jl disagrees with me. Have not had time to check libieeep1788 yet.)

@Chris00 Chris00 force-pushed the cancel branch 3 times, most recently from 95a717d to 8d2d49c Compare February 20, 2022 18:13
@Chris00
Copy link
Contributor Author

Chris00 commented Feb 20, 2022

(Well, I cloned libieeep1788 and tried to compile it but I got errors. Not sure to pursue it much further.)

@unageek
Copy link
Owner

unageek commented Feb 21, 2022

what should the value of const_interval!(f64::MAX, f64::MAX).cancel_minus(const_interval!(f64::MIN, f64::MIN)) be?

IMHO it should be const_interval!(f64::MAX, f86::INFINITY).

I agree with you. To be clear, let me explain my understanding.

cancelMinus([MAX, MAX], [MIN, MIN]) returns the tightest interval [a, b] s.t. [MIN, MIN] + [a, b] = [MIN + a, MIN + b] ⊇ [MAX, MAX].

Then, a is MAX since MIN + MAX = 0.0MAX, but the next value +∞ cannot be the lower bound of an interval.

b is +∞ since MIN + +∞ = +∞MAX, but the previous value MAX does not satisfy the relation as MIN + MAX = 0.0.

@unageek
Copy link
Owner

unageek commented Feb 22, 2022

I haven't understood the code thoroughly yet, but I wonder why add_rn/sub_rn are used in 2Sum. Could you elaborate on it, please?

@Chris00
Copy link
Contributor Author

Chris00 commented Feb 24, 2022

The idea of the code is to (first) compare the widths of the intervals x and y to check the condition width(x) ≥ width(y). (I choose the widths first because I imagined that this function would rather be used for intervals with reasonable widths — as opposed to very large intervals for which it is less useful.) When a strict inequality holds between the rounded widths, the same strict inequality holds between then exact widths (and the results is sub_ru(x, y) of ENTIRE). When the rounded widths are equal, one computes the “residuals” to decide the order of the exact widths (unless the widths are rounded to +∞ in which case we rather compare the exact z̲ and z̅). The “residuals” are computed with the 2Sum algorithm (which only works with round to nearest mode (although it is quite robust and one may possibly use that paper to improve the algo for our purposes). The code follows closely the 2sum algorithm for x̅ - x̲ and the same for y computed in parallel — whence the sub_rn with the shuffle...

@unageek
Copy link
Owner

unageek commented Feb 24, 2022

Oops, I thought that _rn stands for "round down". Thank you for clarifying the algorithm. I'll check the code again.

@Chris00
Copy link
Contributor Author

Chris00 commented Feb 24, 2022

Just renamed hadd as hadd_rn for homogeneity and clarity.

@Chris00
Copy link
Contributor Author

Chris00 commented Feb 24, 2022

I documented the _rn functions (on X86).

@unageek
Copy link
Owner

unageek commented Feb 25, 2022

I have added further annotations to the code. Could you take a look and “merge” it if it is OK?

https://gist.github.com/unageek/0dda441af9dcc782f4bd7d19a05aab52

@unageek
Copy link
Owner

unageek commented Feb 25, 2022

I also have decided to apply cargo fmt to the codes under itf1788_tests (see 130f6b9, sorry for the timing). I hope it is not too difficult to rebase the branch onto main.

@Chris00
Copy link
Contributor Author

Chris00 commented Feb 27, 2022

I have incorporated your comments. I sometimes felt they were a bit redundant to the code but, if they helped you, they may also help others so I left all of them.

@unageek
Copy link
Owner

unageek commented Feb 27, 2022

LGTM! Thank you very much!

@unageek unageek merged commit f0ead11 into unageek:main Feb 27, 2022
@Chris00 Chris00 deleted the cancel branch March 2, 2022 22:11
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

Successfully merging this pull request may close these issues.

None yet

3 participants