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

printf("%# 01.1g", 9.8) yields decimal rather than exponential formatting #156

Open
eyalroz opened this issue Jul 10, 2023 · 10 comments
Open
Assignees
Labels
bug Something isn't working resolved-on-develop A changeset fixing this issue has been commiutted to the development branch

Comments

@eyalroz
Copy link
Owner

eyalroz commented Jul 10, 2023

printf("%# 01.1g", 9.8) should yield 1.e01, but yields 10.; this is likely due to a mis-evaluation of how many characters the exponential-format takes up, resulting in a mistaken decision to use decimal mode.

See discussion of this issue on StackOverflow.

@eyalroz eyalroz added the bug Something isn't working label Jul 10, 2023
@eyalroz eyalroz self-assigned this Jul 10, 2023
@klux21
Copy link

klux21 commented Aug 7, 2023

I just noticed the same issue in my own callback_printf project here. It didn't care any rounding before taking the exponent of the value for the checker rule as well. The MSVC 8.0 compiler prints " 1.e+001" in this case. (It prints always 3 digits for the exponent.) But the exponent always requires at least 2 digits and a sign according to the C Standard. So the output it should be " 1.e+01".

@eyalroz
Copy link
Owner Author

eyalroz commented Aug 7, 2023

@klux21 : A PR would be appreciated. Otherwise, I'll need to find the time to look into this.

@klux21
Copy link

klux21 commented Aug 7, 2023

Sorry I don't use this implementation but had a look at it only. callback_printf is according to my own tests faster and prints floating points at higher precision. It supports long doubles and %a already and converts wchar_t to UTF8. But it's a new thing and I'm still lacking any feedback. So I had a look at the problems here for improving my test base a little bit.

@eyalroz
Copy link
Owner Author

eyalroz commented Aug 7, 2023

@klux21 : Well, this repository has different design goals:

  • Small size
  • Short-but-readable code
  • Ability to turn features on and off, like floating point support and such
  • No functionality beyond the printf family of functions, i.e. not UTF-8 conversion and such.

so we're in a different point in the design space. But I should definitely fix this, and implement %a at some point.

If you have benchmark code, I would like to see that, to perhaps include it as one of the tests as a courtesy to users. OTOH, this library has a more lenient license, so we'd need to work through that.

@klux21
Copy link

klux21 commented Aug 7, 2023

You are right the design goals are different ones. The 'benchmark' was just printing some random different format strings in a tiny loop and measuring time of writing an output string. Nothing that anyone could brag with. But you are right - a more real world benchmark of different format strings would be nice.
My goal for callback_printf was just a fast and full featured replacement of sprintf for ensuring a fast logging on all platforms without caring different format strings any more. Floats are only supported because I'm using a simple %f in just one of my logs only. May be it's even wrong to share the sources except for preventing additional software patents on floating point to decimal conversions. But regarding the license - should I take that as an offer of paying for the development of that lib? I would really like to change it afterwards. 😁

@eyalroz
Copy link
Owner Author

eyalroz commented Aug 7, 2023

should I take that as an offer of paying for the development of that lib?

No, I just meant that if you wanted to share benchmark code that's GPL for me to also have in this library, I would need your agreement to publish it under an MIT license. But it's a moot point given what you've said.

@klux21
Copy link

klux21 commented Aug 8, 2023

My test wasn't a benchmark or worth calling it that. I did execute that for some few different format strings for writing some strings, hexadecimal long ints and some doubles with %e. But we need a little test for seeing what's slow or broken in comparison to other implementations. For this I did create a little one out of my regression test but replaced the Microsoft I64 prefixes with ll for making the glibc more happy. The results draw a more mixed picture and are a little bit puzzling as well. I did add a GPL header for not puzzling people but it's not important for a test like that either which is a developer tool only. You'll find it in my repository. (It seems that you have been the first living visitor there btw. 😁) Put your sources in a printf subfolder and execute the file bench_vsprintf.c as a shell script. It tests the vsprintf and vsnprintf versions only because it's a bit easier to do and sprintf as well as snprintf are usually just calling those either. The results may vary depending on the platform, the compiler and the optimization flag.
But have a look at it yourself. The test prints the calling parameters and the expected result on top of every test case and below of that the time in microseconds and the output values of the different implementations.

What's up with that 0.5 rounding of your lib and the one of the compilers? Why 0? Rounding 1.5 results in 2 as expected.
You'll find a test case for this problem here as well. Your head version here prints a lot of zeros in case of that input.

@klux21
Copy link

klux21 commented Aug 8, 2023

The test cases a pretty random and we need a better structured list of test cases. But still better then nothing. 😁
I did fix my %a issues now. I guess it's %a for pain in the a... because it mixes binary, hexadecimal and decimal conversions.

@klux21
Copy link

klux21 commented Aug 13, 2023

I did rename the benchmark to vsprintf_bench.c now and removed the inclusion of your sources for getting rid of the dependency.
The last tag that includes your project for a direct comparison is callback_printf_1.0.6 .
The benchmark is GPL but I doubt that anyone needs that test in any kind of closed source software either.

eyalroz pushed a commit that referenced this issue Dec 12, 2023
eyalroz pushed a commit that referenced this issue Jul 19, 2024
eyalroz pushed a commit that referenced this issue Jul 19, 2024
@eyalroz
Copy link
Owner Author

eyalroz commented Jul 23, 2024

The problem was, that we were making the decision regarding whether to fall back to decimal-only mode too soon. In some cases, separating the exponential and decimal components results in a roll-over due to the limited precision, increasing the exponent; and this increase can take us past the threshold of choice between the two modes.

So, I've delayed that decision, at the cost of having to have two separate call sites for get_components() :-(

@eyalroz eyalroz added the resolved-on-develop A changeset fixing this issue has been commiutted to the development branch label Jul 23, 2024
eyalroz added a commit that referenced this issue Jul 24, 2024
… when deciding whether to revert from exponent-mode to decimal-only-mode
eyalroz added a commit that referenced this issue Jul 25, 2024
… when deciding whether to revert from exponent-mode to decimal-only-mode
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working resolved-on-develop A changeset fixing this issue has been commiutted to the development branch
Projects
None yet
Development

No branches or pull requests

2 participants