-
Notifications
You must be signed in to change notification settings - Fork 130
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 macro for consistent sunrealtype formatting #517
base: develop
Are you sure you want to change the base?
Conversation
test/answers
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need to open a PR in the answers repo to merge these updates into main
test/unit_tests/arkode/CXX_serial/ark_test_dahlquist_mri_-1.out
Outdated
Show resolved
Hide resolved
Co-authored-by: David Gardner <[email protected]>
Co-authored-by: David Gardner <[email protected]>
…feature/real-format3
include/sundials/sundials_types.h
Outdated
#if defined(SUNDIALS_SINGLE_PRECISION) | ||
|
||
typedef float sunrealtype; | ||
#define SUN_RCONST(x) x##F | ||
#define SUN_BIG_REAL FLT_MAX | ||
#define SUN_SMALL_REAL FLT_MIN | ||
#define SUN_UNIT_ROUNDOFF FLT_EPSILON | ||
#define SUN_FORMAT_E "% ." SUN_STRING(FLT_DIG) "e" | ||
#define SUN_FORMAT_G "% ." SUN_STRING(FLT_DIG) "g" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking at the updated outputs with "% ."
I think it would be better to have this macro not include %
(i.e., use #define SUN_FORMAT_G "." SUN_STRING(FLT_DIG) "g"
. This way the space after %
can be added locally in the format string when it's needed for alignment (i.e., outputting matrix/vector entries) and in other cases (like PrintAllStats
) the space can be excluded.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree the new output file format is not ideal with double spaces for positive numbers. What about this?
#define SUN_FORMAT_E "% ." SUN_STRING(FLT_DIG) "e"
#define SUN_FORMAT_G "%." SUN_STRING(FLT_DIG) "g"
Note I just removed a space from the g format. We can use SUN_FORMAT_E
for cases where alignment is needed (I think this is already the case), and SUN_FORMAT_G
elsewhere. We could even rename them to SUN_FORMAT_ALIGNED
and SUN_FORMAT
. I like the simplicity and consistency of just two macros which contain the entire format specifier. Let's discuss at tomorrow's meeting.
Replaces #498 after v7.1.0 update