-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Added Float32 and Float64 consts #4787
Conversation
Huh, travis fails on 32 bits with strange error. No idea what could i broke (and no 32-bit machine to test) - |
Try to not change MIN and MAX. |
Still broken on 32-bits. No ideas. |
Well, working now. I'll try to revert |
22ff674
to
412b508
Compare
Reverted, CI is green 😄 |
It would be great if you could specify the float constants as hexadecimal then use |
@RX14 in that case, should there also be some specs ensuring that |
@konovod yes. |
done, but there is an error on 32-bits travis again. If it happens on other PRs too it's ok (well, not ok, but unrelated), but if only on this - perhaps there is some problem with it? |
@konovod what is the status of this PR? |
I don't see anything preventing it from merge. Travis 32-bit was failing with apparently irrelevant error, perhaps will be green if restarted. But i forgot how to restart CI - should I force push the same commit? |
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.
Perhaps the specs should be the other way around: specify the real numbers as constants then check them against the hex values in the specs.
src/float.cr
Outdated
# value is | ||
EPSILON = 0x34000000_u32.unsafe_as(Float32) # 1.19209290e-07_f32 | ||
# The number of decimal digits that can be represented without losing precision | ||
DIGITS = 15 |
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.
This should be 6.
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.
nice catch, my bad!
This might sounds like a dumb question but are these set at compile time or runtime? seem to me things like |
Even if |
src/float.cr
Outdated
# value is | ||
EPSILON = 1.19209290e-07_f32 | ||
# The number of decimal digits that can be represented without losing precision | ||
DIGITS = 15 |
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.
Still 6
:)
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.
huh
src/float.cr
Outdated
# Largest finite value | ||
MAX = 3.40282347e+38_f32 | ||
# The machine epsilon (difference between 1.0 and the next representable value) | ||
# value is |
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.
Probably leftover from a change?
Thanks @konovod ! |
Fixes #4785
Adds
EPSILON
,DIGITS
,RADIX
,MANT_DIG
,MIN_EXP
,MAX_EXP
,MIN_10_EXP
,MAX_10_EXP
andMIN_POSITIVE
consts to Float32 and Float64.I've renamed DIG -> DIGITS as it imho looks clearer this way. For a
ROUNDS
- I don't know how to get its value and is it really needed, so skipped it.If the constants are different for some platforms, they of course should be moved to platform-specific files, but i think they are currently same for all supported platforms.