Opened 7 years ago
Closed 7 years ago
#19428 closed enhancement (fixed)
Add support for MPFR_RNDA rounding mode
Reported by: | jdemeyer | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-7.0 |
Component: | basic arithmetic | Keywords: | |
Cc: | Merged in: | ||
Authors: | Jeroen Demeyer | Reviewers: | Vincent Delecroix, Marc Mezzarobba |
Report Upstream: | N/A | Work issues: | |
Branch: | 9c58daa (Commits, GitHub, GitLab) | Commit: | 9c58daa5a93b1ffe3b7ba72287453828a4637a5f |
Dependencies: | Stopgaps: |
Description (last modified by )
This rounding mode is "new" since MPFR 3.
Also use MPFR 3 names:
GMP_RNDx
->MPFR_RNDx
mp_prec_t
->mpfr_prec_t
mp_rnd_t
->mpfr_rnd_t
Change History (28)
comment:1 Changed 7 years ago by
Branch: | → u/jdemeyer/add_support_for_mpfr_rnda_rounding_mode |
---|
comment:2 Changed 7 years ago by
Commit: | → c8ab56233c3141da1ea37309073384534f866800 |
---|---|
Description: | modified (diff) |
Status: | new → needs_review |
comment:3 Changed 7 years ago by
Commit: | c8ab56233c3141da1ea37309073384534f866800 → 2a1dd8947de11919a40824b7ccad80fc2331372c |
---|
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
2a1dd89 | Introduce MPFR_RNDA, rename GMP -> MPFR rounding modes
|
comment:4 Changed 7 years ago by
Commit: | 2a1dd8947de11919a40824b7ccad80fc2331372c → a12282421aff5c5b535643c67ac46a9db5d09f1d |
---|
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
a122824 | Introduce MPFR_RNDA, rename GMP -> MPFR rounding modes
|
comment:5 Changed 7 years ago by
Description: | modified (diff) |
---|
comment:6 Changed 7 years ago by
Reviewers: | → Vincent Delecroix |
---|---|
Status: | needs_review → needs_work |
Wrong syntax
+ TESTS:: + + Test the various rounding modes::
comment:7 Changed 7 years ago by
Commit: | a12282421aff5c5b535643c67ac46a9db5d09f1d → 649048e39f7d4d86d89d1dc741173c9b132a0ae4 |
---|
Branch pushed to git repo; I updated commit sha1. New commits:
649048e | Fix documentation formatting
|
comment:8 Changed 7 years ago by
Status: | needs_work → needs_review |
---|
comment:9 Changed 7 years ago by
Commit: | 649048e39f7d4d86d89d1dc741173c9b132a0ae4 → 15c471c7bfc2f88355624534c34565015336768a |
---|
comment:10 follow-up: 11 Changed 7 years ago by
RNDA is marked experimental in the MPFR manual, and Vincent Lefèvre recently insisted on mpfr-devel that it "is not part of the API" and "could disappear entirely without notice", so I'm not sure it is such a good idea to start supporting it unless we have a real use for it.
comment:11 follow-up: 12 Changed 7 years ago by
Replying to mmezzarobba:
RNDA is marked experimental in the MPFR manual
I cannot find anything like that in the manual of mpfr-3.1.3 (are you using an outdated version?). What is your reference?
comment:12 follow-up: 13 Changed 7 years ago by
Replying to jdemeyer:
Replying to mmezzarobba:
RNDA is marked experimental in the MPFR manual
I cannot find anything like that in the manual of mpfr-3.1.3 (are you using an outdated version?). What is your reference?
First paragraph of Section 4.4 (or just grep for "experimental").
Edit: oh, no, sorry, it was marked as such in the manual of MPFR 3.0, but that no longer seems to be the case with the current release. Well, then I don't know why Vincent said that.
comment:13 follow-up: 14 Changed 7 years ago by
Replying to mmezzarobba:
Edit: oh, no, sorry, it was marked as such in the manual of MPFR 3.0
Well, it was a new feature in MPFR 3.0, so it's not surprising that it was experimental initially.
Do you have a link to the post of Vincent Lefèvre?
comment:14 Changed 7 years ago by
Replying to jdemeyer:
Replying to mmezzarobba:
Edit: oh, no, sorry, it was marked as such in the manual of MPFR 3.0
Well, it was a new feature in MPFR 3.0, so it's not surprising that it was experimental initially.
Do you have a link to the post of Vincent Lefèvre?
https://sympa.inria.fr/sympa/arc/mpfr/2015-12/msg00071.html
I just saw he was talking abound MPFR_RNDNA, which I didn't even know existed. False alarm, sorry.
comment:15 follow-up: 16 Changed 7 years ago by
If you're replacing GMP_RNDx
by MPFR_RNDx
everywhere, I guess you may also want to replace mp_rnd_t
by mpfr_rnd_t
(and perhaps mp_prec_t
by mpfr_prec_t
) in mpfr.pxd
. Unless there is a particular reason not to do it?
comment:16 follow-up: 17 Changed 7 years ago by
Replying to mmezzarobba:
If you're replacing
GMP_RNDx
byMPFR_RNDx
everywhere, I guess you may also want to replacemp_rnd_t
bympfr_rnd_t
(and perhapsmp_prec_t
bympfr_prec_t
) inmpfr.pxd
. Unless there is a particular reason not to do it?
There is no particular reason, I just didn't bother. Will you review it if I make those replacements?
comment:17 Changed 7 years ago by
Replying to jdemeyer:
There is no particular reason, I just didn't bother. Will you review it if I make those replacements?
I don't have much time, but yes, I can try to review it (hopefully next Monday) in any case.
comment:18 Changed 7 years ago by
Commit: | 15c471c7bfc2f88355624534c34565015336768a → 16257d8bf5749e20642f21a2b1b122246e4b8630 |
---|
Branch pushed to git repo; I updated commit sha1. New commits:
16257d8 | Use mpfr_rnd_t and mpfr_prec_t
|
comment:19 Changed 7 years ago by
Description: | modified (diff) |
---|
comment:20 Changed 7 years ago by
Milestone: | sage-6.10 → sage-7.0 |
---|---|
Reviewers: | Vincent Delecroix → Vincent Delecroix, Marc Mezzarobba, |
comment:21 Changed 7 years ago by
Reviewers: | Vincent Delecroix, Marc Mezzarobba, → Vincent Delecroix, Marc Mezzarobba |
---|
comment:22 Changed 7 years ago by
I added two commits with minor improvements (or so I hope!) in u/mmezzarobba/add_support_for_mpfr_rnda_rounding_mode
. Please merge that branch if you agree with the changes. Whether you merge them or not, feel free to set the ticket to positive review if all tests pass.
comment:23 Changed 7 years ago by
Branch: | u/jdemeyer/add_support_for_mpfr_rnda_rounding_mode → u/mmezzarobba/add_support_for_mpfr_rnda_rounding_mode |
---|---|
Commit: | 16257d8bf5749e20642f21a2b1b122246e4b8630 → 9c58daa5a93b1ffe3b7ba72287453828a4637a5f |
comment:24 follow-up: 25 Changed 7 years ago by
Your changes look good to me. Did you run doctests with your changes?
comment:25 Changed 7 years ago by
Replying to jdemeyer:
Your changes look good to me. Did you run doctests with your changes?
Only the tests in rings/
and libs/
.
comment:26 Changed 7 years ago by
To be sure, I will run make ptestlong
and set positive review if it passes.
comment:27 Changed 7 years ago by
Status: | needs_review → positive_review |
---|
comment:28 Changed 7 years ago by
Branch: | u/mmezzarobba/add_support_for_mpfr_rnda_rounding_mode → 9c58daa5a93b1ffe3b7ba72287453828a4637a5f |
---|---|
Resolution: | → fixed |
Status: | positive_review → closed |
New commits:
Introduce MPFR_RNDA, rename GMP -> MPFR rounding modes