-
-
Notifications
You must be signed in to change notification settings - Fork 18.1k
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
BUG: float conversion in read_csv is inaccurate for precise input #8002
Comments
how about an option for this (default is turned off), may |
That could work--our decision in AstroPy was also to use a parameter named |
I would like to leave the existing code in-place. No objection to adding new options like this though. I don't think the intermediate (using |
Just to add another voice: I also ran into this issue recently. We were serialising data to disk via |
ok @amras1 we'll put this on the table for a new option. |
Sounds good--I have the altered |
I suppose you could have 3 levels for |
This is coming from #2566, which noted that the
xstrtod()
method used byread_csv
doesn't agree with standard numpy float conversion. I see that the priority is speed of parsing over complete accuracy (within 0.5 units in the last place, or ULP), but there are two issues here that (at least to me) actually appear to be buggy:xstrtod()
is off by more than 1 ULP, although these don't come up often.strtod()
, but the error in conversion increases linearly as the number of supplied significant figures increases. With 30 significant figures, the error in conversion can potentially be over 7 ULP.Here is an IPython notebook analyzing the accuracy of
xstrtod()
. I think there are two problems here:xstrtod()
keeps reading digits after the 17th, none of which should matter for conversion, and the scaling step at the end produces a compounded error by repeatedly multiplying/dividing by powers of 10. I have a solution for AstroPy that seems to fix these issues, so I can open a PR if it's agreed thatxstrtod()
should be changed.The text was updated successfully, but these errors were encountered: