Skip to content

py/parsenum: Ensure that trailing zeros lead to identical results. #8980

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

Closed
wants to merge 1 commit into from

Conversation

dpwe
Copy link
Contributor

@dpwe dpwe commented Jul 29, 2022

parsenum would calculate "1e-20" as 1.0 * pow(10, -20), and "1.000e-20" as 1000.0 * pow(10, -23); in certain cases, this could make seemingly-identical values compare as not equal. This patch watches for trailing zeros as a special case, and ignores them when appropriate, so "1.000e-20" is also calculated as 1.0 * pow(10, -20).

For the unix build, a test case is print(1e-20 - 1.000000000000e-20) which returns -8.077936e-28 under MICROPY_FLOAT_IMPL_FLOAT build, and -1.504632769052528e-36 under MICROPY_FLOAT_IMPL_DOUBLE. With this PR, the result is 0.0 in both cases.

This surfaced while trying to align the behavior of py/parsenum and py/formatfloat.

Signed-off-by: Dan Ellis dan.ellis@gmail.com

@dpwe dpwe force-pushed the trailing-zeros branch from 063c634 to ab2b0be Compare July 29, 2022 03:32
@codecov-commenter
Copy link

codecov-commenter commented Jul 29, 2022

Codecov Report

❗ No coverage uploaded for pull request base (master@963e599). Click here to learn what that means.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##             master    #8980   +/-   ##
=========================================
  Coverage          ?   98.37%           
=========================================
  Files             ?      156           
  Lines             ?    20293           
  Branches          ?        0           
=========================================
  Hits              ?    19963           
  Misses            ?      330           
  Partials          ?        0           
Impacted Files Coverage Δ
py/parsenum.c 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 963e599...f7828c9. Read the comment docs.

@dpwe dpwe force-pushed the trailing-zeros branch from ab2b0be to f7828c9 Compare July 29, 2022 03:50
dpwe added a commit to dpwe/micropython that referenced this pull request Jul 29, 2022
Formerly, ```formatfloat``` struggled with formatting exact powers of 10
as created by ```parsenum``` because of small differences in how those
powers of 10 were calculated: ```formatfloat``` multiplied together a few
values of 10^(2^Y) from a lookup table, but ```parsenum``` used
```pow(10, X)```.  This was mitigated in micropython#8905 by adding 2eps of extra
margin when comparing values ("aggressive rounding up").

This patch instead eliminates the discrepency by using ```pow()``` in
```formatfloat``` also.  This eliminates the need for the 2eps margin, as
well as the lookup tables.  It is likely slower to run, however.

In addition, this patch directly estimates the power-of-10 exponent from
the power-of-2 exponent in the floating-point representation.

This change surfaced a previously-noted problem with parsing numbers
including trailing zeros.  The fix to that problem, micropython#8980, is included
in this PR to ensure the tests pass.

Signed-off-by: Dan Ellis <dan.ellis@gmail.com>
dpwe added a commit to dpwe/micropython that referenced this pull request Jul 29, 2022
Formerly, ```formatfloat``` struggled with formatting exact powers of 10
as created by ```parsenum``` because of small differences in how those
powers of 10 were calculated: ```formatfloat``` multiplied together a few
values of 10^(2^Y) from a lookup table, but ```parsenum``` used
```pow(10, X)```.  This was mitigated in micropython#8905 by adding 2eps of extra
margin when comparing values ("aggressive rounding up").

This patch instead eliminates the discrepency by using ```pow()``` in
```formatfloat``` also.  This eliminates the need for the 2eps margin, as
well as the lookup tables.  It is likely slower to run, however.

In addition, this patch directly estimates the power-of-10 exponent from
the power-of-2 exponent in the floating-point representation.

This change surfaced a previously-noted problem with parsing numbers
including trailing zeros.  The fix to that problem, micropython#8980, is included
in this PR to ensure the tests pass.

Signed-off-by: Dan Ellis <dan.ellis@gmail.com>
dpwe added a commit to dpwe/micropython that referenced this pull request Jul 29, 2022
Formerly, ```formatfloat``` struggled with formatting exact powers of 10
as created by ```parsenum``` because of small differences in how those
powers of 10 were calculated: ```formatfloat``` multiplied together a few
values of 10^(2^Y) from a lookup table, but ```parsenum``` used
```pow(10, X)```.  This was mitigated in micropython#8905 by adding 2eps of extra
margin when comparing values ("aggressive rounding up").

This patch instead eliminates the discrepency by using ```pow()``` in
```formatfloat``` also.  This eliminates the need for the 2eps margin, as
well as the lookup tables.  It is likely slower to run, however.

In addition, this patch directly estimates the power-of-10 exponent from
the power-of-2 exponent in the floating-point representation.

This change surfaced a previously-noted problem with parsing numbers
including trailing zeros.  The fix to that problem, micropython#8980, is included
in this PR to ensure the tests pass.

Signed-off-by: Dan Ellis <dan.ellis@gmail.com>
dpwe added a commit to dpwe/micropython that referenced this pull request Jul 29, 2022
Formerly, ```formatfloat``` struggled with formatting exact powers of 10
as created by ```parsenum``` because of small differences in how those
powers of 10 were calculated: ```formatfloat``` multiplied together a few
values of 10^(2^Y) from a lookup table, but ```parsenum``` used
```pow(10, X)```.  This was mitigated in micropython#8905 by adding 2eps of extra
margin when comparing values ("aggressive rounding up").

This patch instead removes the discrepency by using ```pow()``` in
```formatfloat``` also.  This eliminates the need for the 2eps margin, as
well as the lookup tables.  It is likely slower to run, however.

In addition, this patch directly estimates the power-of-10 exponent from
the power-of-2 exponent in the floating-point representation.

This change surfaced a previously-noted problem with parsing numbers
including trailing zeros.  The fix to that problem, micropython#8980, is included
in this PR to ensure the tests pass.

Signed-off-by: Dan Ellis <dan.ellis@gmail.com>
dpwe added a commit to dpwe/micropython that referenced this pull request Aug 2, 2022
Formerly, ```formatfloat``` struggled with formatting exact powers of 10
as created by ```parsenum``` because of small differences in how those
powers of 10 were calculated: ```formatfloat``` multiplied together a few
values of 10^(2^Y) from a lookup table, but ```parsenum``` used
```pow(10, X)```.  This was mitigated in micropython#8905 by adding 2eps of extra
margin when comparing values ("aggressive rounding up").

This patch instead removes the discrepency by using ```pow()``` in
```formatfloat``` also.  This eliminates the need for the 2eps margin, as
well as the lookup tables.  It is likely slower to run, however.

In addition, this patch directly estimates the power-of-10 exponent from
the power-of-2 exponent in the floating-point representation.

This change surfaced a previously-noted problem with parsing numbers
including trailing zeros.  The fix to that problem, micropython#8980, is included
in this PR to make the tests pass.

Signed-off-by: Dan Ellis <dan.ellis@gmail.com>
dpwe added a commit to dpwe/micropython that referenced this pull request Aug 2, 2022
Formerly, ```formatfloat``` struggled with formatting exact powers of 10
as created by ```parsenum``` because of small differences in how those
powers of 10 were calculated: ```formatfloat``` multiplied together a few
values of 10^(2^Y) from a lookup table, but ```parsenum``` used
```pow(10, X)```.  This was mitigated in micropython#8905 by adding 2eps of extra
margin when comparing values ("aggressive rounding up").

This patch instead removes the discrepency by using ```pow()``` in
```formatfloat``` also.  This eliminates the need for the 2eps margin, as
well as the lookup tables.  It is likely slower to run, however.

In addition, this patch directly estimates the power-of-10 exponent from
the power-of-2 exponent in the floating-point representation.

This change surfaced a previously-noted problem with parsing numbers
including trailing zeros.  The fix to that problem, micropython#8980, is included
in this PR to make the tests pass.

Signed-off-by: Dan Ellis <dan.ellis@gmail.com>
dpwe added a commit to dpwe/micropython that referenced this pull request Aug 2, 2022
Formerly, ```formatfloat``` struggled with formatting exact powers of 10
as created by ```parsenum``` because of small differences in how those
powers of 10 were calculated: ```formatfloat``` multiplied together a few
values of 10^(2^Y) from a lookup table, but ```parsenum``` used
```pow(10, X)```.  This was mitigated in micropython#8905 by adding 2eps of extra
margin when comparing values ("aggressive rounding up").

This patch instead removes the discrepency by using ```pow()``` in
```formatfloat``` also.  This eliminates the need for the 2eps margin, as
well as the lookup tables.  It is likely slower to run, however.

In addition, this patch directly estimates the power-of-10 exponent from
the power-of-2 exponent in the floating-point representation.

This change surfaced a previously-noted problem with parsing numbers
including trailing zeros.  The fix to that problem, micropython#8980, is included
in this PR to make the tests pass.

Signed-off-by: Dan Ellis <dan.ellis@gmail.com>
dpwe added a commit to dpwe/micropython that referenced this pull request Aug 2, 2022
Formerly, ```formatfloat``` struggled with formatting exact powers of 10
as created by ```parsenum``` because of small differences in how those
powers of 10 were calculated: ```formatfloat``` multiplied together a few
values of 10^(2^Y) from a lookup table, but ```parsenum``` used
```pow(10, X)```.  This was mitigated in micropython#8905 by adding 2eps of extra
margin when comparing values ("aggressive rounding up").

This patch instead removes the discrepency by using ```pow()``` in
```formatfloat``` also.  This eliminates the need for the 2eps margin, as
well as the lookup tables.  It is likely slower to run, however.

In addition, this patch directly estimates the power-of-10 exponent from
the power-of-2 exponent in the floating-point representation.

This change surfaced a previously-noted problem with parsing numbers
including trailing zeros.  The fix to that problem, micropython#8980, is included
in this PR to make the tests pass.

Signed-off-by: Dan Ellis <dan.ellis@gmail.com>
dpwe added a commit to dpwe/micropython that referenced this pull request Aug 2, 2022
Formerly, ```formatfloat``` struggled with formatting exact powers of 10
as created by ```parsenum``` because of small differences in how those
powers of 10 were calculated: ```formatfloat``` multiplied together a few
values of 10^(2^Y) from a lookup table, but ```parsenum``` used
```pow(10, X)```.  This was mitigated in micropython#8905 by adding 2eps of extra
margin when comparing values ("aggressive rounding up").

This patch instead removes the discrepency by using ```pow()``` in
```formatfloat``` also.  This eliminates the need for the 2eps margin, as
well as the lookup tables.  It is likely slower to run, however.

In addition, this patch directly estimates the power-of-10 exponent from
the power-of-2 exponent in the floating-point representation.

This change surfaced a previously-noted problem with parsing numbers
including trailing zeros.  The fix to that problem, micropython#8980, is included
in this PR to make the tests pass.

Signed-off-by: Dan Ellis <dan.ellis@gmail.com>
dpwe added a commit to dpwe/micropython that referenced this pull request Aug 2, 2022
Formerly, ```formatfloat``` struggled with formatting exact powers of 10
as created by ```parsenum``` because of small differences in how those
powers of 10 were calculated: ```formatfloat``` multiplied together a few
values of 10^(2^Y) from a lookup table, but ```parsenum``` used
```pow(10, X)```.  This was mitigated in micropython#8905 by adding 2eps of extra
margin when comparing values ("aggressive rounding up").

This patch instead removes the discrepency by using ```pow()``` in
```formatfloat``` also.  This eliminates the need for the 2eps margin, as
well as the lookup tables.  It is likely slower to run, however.

In addition, this patch directly estimates the power-of-10 exponent from
the power-of-2 exponent in the floating-point representation.

This change surfaced a previously-noted problem with parsing numbers
including trailing zeros.  The fix to that problem, micropython#8980, is included
in this PR to make the tests pass.

Signed-off-by: Dan Ellis <dan.ellis@gmail.com>
dpwe added a commit to dpwe/micropython that referenced this pull request Aug 4, 2022
Formerly, ```formatfloat``` struggled with formatting exact powers of 10
as created by ```parsenum``` because of small differences in how those
powers of 10 were calculated: ```formatfloat``` multiplied together a few
values of 10^(2^Y) from a lookup table, but ```parsenum``` used
```pow(10, X)```.  This was mitigated in micropython#8905 by adding 2eps of extra
margin when comparing values ("aggressive rounding up").

This patch instead removes the discrepency by using ```pow()``` in
```formatfloat``` also.  This eliminates the need for the 2eps margin, as
well as the lookup tables.  It is likely slower to run, however.

In addition, this patch directly estimates the power-of-10 exponent from
the power-of-2 exponent in the floating-point representation.

This change surfaced a previously-noted problem with parsing numbers
including trailing zeros.  The fix to that problem, micropython#8980, is included
in this PR to make the tests pass.

Signed-off-by: Dan Ellis <dan.ellis@gmail.com>
dpwe added a commit to dpwe/micropython that referenced this pull request Aug 4, 2022
Formerly, ```formatfloat``` struggled with formatting exact powers of 10
as created by ```parsenum``` because of small differences in how those
powers of 10 were calculated: ```formatfloat``` multiplied together a few
values of 10^(2^Y) from a lookup table, but ```parsenum``` used
```pow(10, X)```.  This was mitigated in micropython#8905 by adding 2eps of extra
margin when comparing values ("aggressive rounding up").

This patch instead removes the discrepency by using ```pow()``` in
```formatfloat``` also.  This eliminates the need for the 2eps margin, as
well as the lookup tables.  It is likely slower to run, however.

In addition, this patch directly estimates the power-of-10 exponent from
the power-of-2 exponent in the floating-point representation.

This change surfaced a previously-noted problem with parsing numbers
including trailing zeros.  The fix to that problem, micropython#8980, is included
in this PR to make the tests pass.

Signed-off-by: Dan Ellis <dan.ellis@gmail.com>
@dpgeorge dpgeorge added the py-core Relates to py/ directory in source label Aug 11, 2022
@dpgeorge
Copy link
Member

Code size diff for this PR:

   bare-arm:    +0 +0.000% 
minimal x86:    +0 +0.000% 
   unix x64:   +96 +0.016% 
unix nanbox:  +160 +0.029% 
      stm32:  +128 +0.032% PYBV10
     cc3200:    -8 -0.004% 
    esp8266:  +172 +0.025% GENERIC
      esp32:  +136 +0.009% GENERIC
     mimxrt:  +120 +0.034% TEENSY40
 renesas-ra:  +128 +0.020% RA6M2_EK
        nrf:  +112 +0.060% pca10040
        rp2:  +128 +0.025% PICO
       samd:  +128 +0.091% ADAFRUIT_ITSYBITSY_M4_EXPRESS

dpwe added a commit to dpwe/micropython that referenced this pull request Aug 11, 2022
Formerly, ```formatfloat``` struggled with formatting exact powers of 10
as created by ```parsenum``` because of small differences in how those
powers of 10 were calculated: ```formatfloat``` multiplied together a few
values of 10^(2^Y) from a lookup table, but ```parsenum``` used
```pow(10, X)```.  This was mitigated in micropython#8905 by adding 2eps of extra
margin when comparing values ("aggressive rounding up").

This patch instead removes the discrepency by using ```pow()``` in
```formatfloat``` also.  This eliminates the need for the 2eps margin, as
well as the lookup tables.  It is likely slower to run, however.

In addition, this patch directly estimates the power-of-10 exponent from
the power-of-2 exponent in the floating-point representation.

This change surfaced a previously-noted problem with parsing numbers
including trailing zeros.  The fix to that problem, micropython#8980, is included
in this PR to make the tests pass.

Signed-off-by: Dan Ellis <dan.ellis@gmail.com>
dpwe added a commit to dpwe/micropython that referenced this pull request Aug 11, 2022
Formerly, ```formatfloat``` struggled with formatting exact powers of 10
as created by ```parsenum``` because of small differences in how those
powers of 10 were calculated: ```formatfloat``` multiplied together a few
values of 10^(2^Y) from a lookup table, but ```parsenum``` used
```pow(10, X)```.  This was mitigated in micropython#8905 by adding 2eps of extra
margin when comparing values ("aggressive rounding up").

This patch instead removes the discrepency by using ```pow()``` in
```formatfloat``` also.  This eliminates the need for the 2eps margin, as
well as the lookup tables.  It is likely slower to run, however.

In addition, this patch directly estimates the power-of-10 exponent from
the power-of-2 exponent in the floating-point representation.

This change surfaced a previously-noted problem with parsing numbers
including trailing zeros.  The fix to that problem, micropython#8980, is included
in this PR to make the tests pass.

Signed-off-by: Dan Ellis <dan.ellis@gmail.com>
@dpgeorge
Copy link
Member

Merged in 6cd2e41 (via PR #8985).

@dpgeorge dpgeorge closed this Aug 12, 2022
tannewt pushed a commit to tannewt/circuitpython that referenced this pull request Mar 5, 2024
I tested this by fetching a .txt file repeatedly using curl while
running the fancy camera demo. (100+ times without failure). I also
repeatedly loaded the filesystem view http://.../fs/#/sd/ which worked
10+ times without failure, but does take some time (multiple seconds) to
show a listing with a few dozen files.

(I suspect there's an accidentally quadratic behavior in oofatfs to stat
every file in a directory, because it repeatedly does a linear search of
the directory for the stat information of each file, but that's not an
issue for Right Now(TM))

Closes: micropython#8980
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
py-core Relates to py/ directory in source
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants