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

Add NumberInput.parseFloat() #753

Closed
pjfanning opened this issue Apr 21, 2022 · 9 comments
Closed

Add NumberInput.parseFloat() #753

pjfanning opened this issue Apr 21, 2022 · 9 comments
Milestone

Comments

@pjfanning
Copy link
Member

For v2.14, NumberInput has become the main way to centralise the code for parsing numbers. One inconsistency is that Floats are still parsed using Float.parseFloat. Having a NumberInput.parseFloat that does that means we can change the implementation later to use the fast-double-parser or something similar. See #577

@plokhotnyuk
Copy link

@pjfanning Please ensure that it is used for parsing floats to avoid 1ULP rounding error that happens with v2.13.2

@pjfanning
Copy link
Member Author

@plokhotnyuk I think the jackson issue you describe is because in some cases jackson seems to use Double.parseDouble to parse floats and then later to cast that to a float and that seems to lead to the loss of precision with the "1.199999988079071" case. Float.parse("1.199999988079071") gives 1.1999999f but (float)Double.parse("1.199999988079071") seems to give 1.2f.

@pjfanning
Copy link
Member Author

@plokhotnyuk is 1.2f actually a better answer than 1.999999f - it is actually closer to 1.199999988079071

1.2 - 1.199999988079071 = 1.19209289e-8
1.999999 - 1.199999988079071 = -8.80790711e-8

@plokhotnyuk
Copy link

Exact binary representation of 1.199999988079071 is:

1.0011001100110011001100101111111111111111111111111111110011000011...

When fitting to 32-bit float the properly rounded binary value is:

1.00110011001100110011001

That is 1.19999992847442626953125 when converted back to exact decimal representation.

You can use the following online calculator to check this:
https://www.exploringbinary.com/binary-converter/

@pjfanning
Copy link
Member Author

@cowtowncoder this code in ParserBase (jackson-core) may be contributing to the issue that @plokhotnyuk described with parsing 1.199999988079071 as a float.

    @Override
    public float getFloatValue() throws IOException
    {
        double value = getDoubleValue();
        /* 22-Jan-2009, tatu: Bounds/range checks would be tricky
         *   here, so let's not bother even trying...
         */
        /*
        if (value < -Float.MAX_VALUE || value > MAX_FLOAT_D) {
            _reportError("Numeric value ("+getText()+") out of range of Java float");
        }
        */
        return (float) value;
    }

From my testing, it seems that casting doubles to floats may not get the same result as parsing the text representation directly with Float.parseFloat(String).

@pjfanning
Copy link
Member Author

pjfanning commented Apr 22, 2022

I created #755 for the issue about 1.199999988079071 - I want to keep this issue for the original purpose of updating NumberInput to have a parseFloat(String)

@GedMarc
Copy link

GedMarc commented Apr 22, 2022

Hmm, I may be mistaken, but isn't float inaccurate/non-precise by default, are we absolutely sure the algorithm doesn't go to that value as the closest possible match to the requested value?

Is there any benefit in trying to make an inherent non precise number system precise?

@pjfanning
Copy link
Member Author

I need to go into this with more detail with @plokhotnyuk because I can't reproduce his issue and I'd like to move that discussion to #755 - this issue was originally about a small code refactor

@cowtowncoder cowtowncoder changed the title add NumberInput.parseFloat Add NumberInput.parseFloat() Apr 24, 2022
@cowtowncoder cowtowncoder added this to the 2.14.0 milestone Apr 24, 2022
@cowtowncoder
Copy link
Member

I think this issue specifically is resolved, so closing, but follow-up discussions -- which are related but not directly caused by this -- should proceed.

Put another way: this just adds a method to call; coercions between float, double etc happen in different places in code (mostly ParserBase)

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

No branches or pull requests

4 participants