search

fastfloat / fast_float

Fast and exact implementation of the C++ from_chars functions for number types: 4x to 10x faster than strtod, part of GCC 12, Chromium and WebKit/Safari
Apache License 2.0
1.35k stars 124 forks source link

Allow fast_float to parse strings accepted by the Fortran internal read function. #219

Closed allenbarnett5 closed 12 months ago

allenbarnett5 commented 1 year ago

Hi: I'm working on a program which reads an input file which is ordinarily read by a Fortran program. Numbers in the input are converted to binary with the Fortran internal read function. For example,

CHARACTER(LEN=25) :: CHARS REAL(KIND=8) :: NUMBER READ ( CHARS, '(e25.0)' ) NUMBER

The Fortran internal read accepts a larger variety of formats than std::from_chars. In particular, the 'E' in the exponent is optional (!); it can also be replaced with a 'D'. A leading '+' is also accepted. So, "+1+1" is 10.

I made a few changes to fast_float to support this case. I realize it's somewhat out of scope for a C++ function, but it is useful to me. fast_float is much better than my attempt.

I added a test, but no other CMake infrastructure. Since it does add a condition in parse_number_string, I suppose it slows down the parsing. So, one might want to make it a CMake option and add some conditional #ifdef's. Also, it should #define FASTFLOAT_ALLOWS_LEADING_PLUS. I can add this as an option, but I didn't want to mess with the build infrastructure too much (without approval :-)

This is my first ever Github pull request.

Thanks, Allen

lemire commented 1 year ago

It is reasonable. I don't expect that this PR is likely to cause problems.

lemire commented 1 year ago

It passes our tests, so I think we will make it part of the next release.

I will let people review and react before I merge.

allenbarnett5 commented 1 year ago

That's great! Thank you very much!

lemire commented 1 year ago

@allenbarnett5 Can you react to @mayawarrier comment?

To test whether we allow the fortran format, we need something like fmt & (1<<4) or something... (e.g., fmt == chars_format::fortran).

lemire commented 12 months ago

@allenbarnett5 Please resync with our main branch.

lemire commented 12 months ago

@allenbarnett5 I will merge locally and issue a few fixes.

allenbarnett5 commented 12 months ago

Hi: now I'm confused :-) What should I do?

On Fri, Sep 15, 2023 at 9:21 AM Daniel Lemire @.***> wrote:

@allenbarnett5 https://github.com/allenbarnett5 I will merge locally and issue a few fixes.

— Reply to this email directly, view it on GitHub https://github.com/fastfloat/fast_float/pull/219#issuecomment-1721274354, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABOLFXLKM5OZYE4ESXP2THTX2RI5XANCNFSM6AAAAAA3G7O3BE . You are receiving this because you were mentioned.Message ID: @.***>

lemire commented 12 months ago

@allenbarnett5 Nothing: I merged your PR after making changes.

allenbarnett5 commented 12 months ago

Regarding Maya's comment, I guess if there is more than one bit set in fmt, the test needs to be something like:

(fmt&FASTFLOAT_FORTRANFMT) == FASTFLOAT_FORTRAN

or even

fmt == FASTFLOAT_FORTRAN

so that all the bits are taken into account. Sorry for the confusion.

On Fri, Sep 15, 2023 at 9:58 AM Daniel Lemire @.***> wrote:

@allenbarnett5 https://github.com/allenbarnett5 Nothing: I merged your PR after making changes.

— Reply to this email directly, view it on GitHub https://github.com/fastfloat/fast_float/pull/219#issuecomment-1721328985, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABOLFXO5VGUGTWQYC3RKT7LX2RNIDANCNFSM6AAAAAA3G7O3BE . You are receiving this because you were mentioned.Message ID: @.***>