Exact float conv #13
Open
Exact float conv #13
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 3 commits
November 16, 2019 10:04
Overflow leads to wrong results and can create non-terminating loops in the postgresql backend. Changes: - return ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE when input is too large to be represented as rational - end refinement when the resulting numerator or denominator would not fit into an int32. - added some test cases
- use double negation because conditions with NaN's always evaluate to false - add test case with NaN - some cosmetic changes
This version of rational_in_float() converts the float into an int64/int64 rational and calculates the int32/int32 rational with the least error (where possible reproducing it exactly) using continued fractions. Additionally a funktion rational_limit_denominator() is definined which finds the best approximation for a rational with the denominator not exceeding a given value. One use case is to recover fractions that are stored with limited precision: rational_limit_denominator(0.3333::float, 1000) -> 1/3
|
Can you tell me more about this algorithm? What does it improve, and does it have any disadvantages compared with the existing one? |
begriffs
reviewed
Nov 24, 2019
| /* | ||
| limit_denomintor() uses continued fractions to convert the | ||
| rational n/d into the rational n'/d' with d' < max_denominator | ||
| and n' <= INT32_MAX and smallest |d/n-d'/n'|. |
There was a problem hiding this comment.
Is there a paper or a description of this algorithm you can link to in the comment?
|
Am So., 24. Nov. 2019 um 18:09 Uhr schrieb Joe Nelson <
[email protected]>:
***@***.**** commented on this pull request.
------------------------------
In pg_rational.c
<#13 (comment)>:
> @@ -464,6 +470,81 @@ rational_larger(PG_FUNCTION_ARGS)
************** INTERNAL ***************
*/
+/*
+limit_denomintor() uses continued fractions to convert the
+rational n/d into the rational n'/d' with d' < max_denominator
+and n' <= INT32_MAX and smallest |d/n-d'/n'|.
Is there a paper or a description of this algorithm you can link to in the
comment?
You can find the calculation formula for continued fractions in
https://en.wikipedia.org/wiki/Continued_fraction#Calculating_continued_fraction_representations
the recursive formula for numerator and denominator is explained in
https://en.wikipedia.org/wiki/Continued_fraction#Infinite_continued_fractions_and_convergents
and the program uses variables p/q instead h/k. This algorithm is also used
in the Python fractions module
https://github.com/python/cpython/blob/036fe85bd3e6cd01093d836d71792a1966f961e8/Lib/fractions.py#L227
Calculation ends when either the numerator or the denominator would get too
big, or if we are done because the continued fraction is finished (its
finite) or the fraction is equal to the target number (this can happen
earlier because of finite numerical precision).
Then the secondary convergent with the largest possible numerator and
denominator (with respect to the limits) is calculated and taken as result
if the error is smaller than for the fraction calculated above (
https://en.wikipedia.org/wiki/Continued_fraction#Semiconvergents).
This algorithm is a bit slower than the original one (depends on the number
range, about 20% - 50% slower according to my measurements), but more exact
and numerically quite stable (try to look at what happens with z in the
original algorithm when the iteration gets close to the maximum achievable
numerical precision).
Please feel free to put this text or parts of it into the C function
comment if you are interested.
|
|
Thanks for suggesting this alternate way to do it. I merged your other PR because the changes are smaller, but we can discuss which way to do it on the pg hackers list. In the meantime I'll be releasing another version of the extension to get your fix out there. |
|
Could you rebase this PR please? |
|
@adegert :) |
|
@adegert are you still interested in continuing development on this? |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
An alternative to the current algorithm + new function rational_limit_denominator