Bug #858
floor for TwinFloat can produce ERR::SERIOUS
Description
Fix impl of RingTwinFloatImpl::myFloor so that the following does not happen
ring RR = NewRingTwinFloat(64); RingElem x = one(RR); RingElem smaller = x - power(BigRat(1,2),145); // 1- 2^(-145) BigInt N = floor(smaller); // throws ERR::SERIOUS
Something smilar will be needed for myCeil, and perhaps also for myNearestInt.
Related issues
History
#1
Updated by John Abbott about 8 years ago
Updated by - Related to Bug #853: NearestInt can needlessly throw InsufficientPrecision added
#2
Updated by John Abbott about 8 years ago
- Status changed from New to In Progress
- Assignee set to John Abbott
- % Done changed from 0 to 10
The point is that the candidate value for floor should really be floor of primary component plus a very small epsilon.
Perhaps look at the impl of myIsInteger to see how to proceed.
It's quite hard getting the details right.
What actually failed was a self test inside myFloor; maybe this should be an assertion?
#3
Updated by John Abbott about 8 years ago
One way out of the problem would simply be to reinstate the test:
BigInt N; if (myIsInteger(N,rawx)) return N;
This does seem to be a slightly crude solution, but there may be nothing better.
I'll investigate.
#4
Updated by John Abbott about 8 years ago
- % Done changed from 10 to 20
I am now convinced that myFloor must effectively make a myIsInteger test.
If myIsInteger succeeds then its result is surely the result of myFloor.
If myIsInteger says "no" then the value is "far from" an integer, and taking the floor of the primary component gives the only possible candidate.
If myIsInteger throws InsuffPrec then the value is very close to an integer, i.e. the implicit (open) interval contains an integer, thus there are two possible values for the floor, and I see no justification for choosing between them (i.e. throwing InsuffPrec is the correct behaviour).
#5
Updated by John Abbott about 8 years ago
An ad hoc test behaved as I expected, so I think myFloor (and myCeil are now OK).
Wish I could say the same for myNearestInt... test-OrderedDomain2 is failing again :-(
#6
Updated by John Abbott about 8 years ago
- % Done changed from 20 to 50
I found a bug in myIsInteger, so have written a completely new version (it's quite a nightmare debugging code written directly with mpf_XYZ functions).
Previously it called myIsRational and simply checked whether the denom was 1, but this threw InsuffPrec when it should not have done. Now myIsInteger is separate code.
I have a nasty feeling that myIsRational may still sometimes throw InsuffPrec when it should not... someone else can check that!!
#7
Updated by John Abbott about 8 years ago
- Related to Design #859: Twin-float: comparisons and equality test added
#8
Updated by John Abbott about 8 years ago
- Status changed from In Progress to Feedback
- % Done changed from 50 to 90
I am now happy with the impls; they are simple enough that they are "obviously correct" (I hope!). Also I believe that the impls are reasonably efficient.
Putting issue into feedback.
In principle, it is enough to implement just one of myFloor and myCeil; e.g. myCeil could just be if (integer) return value; else return 1+floor.
I'm not convinced that the loss of readability is sufficiently compensated by the gain in "slickness".
#9
Updated by John Abbott about 8 years ago
- Related to Bug #860: Check impl of RingTwinFloatImpl::myIsRational added
#10
Updated by John Abbott almost 8 years ago
- Status changed from Feedback to Closed
- % Done changed from 90 to 100
- Estimated time set to 3.90 h
I have added a new test (test-bug7.C) for this issue.
Since there have been no problems for 3 months, I am closing the issue.