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.

Sign up for GitHub

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

Jump to bottom

update FastMath to use std::frexp and std::ldexp #333

Closed
BenWibking opened this issue Jul 6, 2023 · 0 comments · Fixed by #471
Closed

update FastMath to use std::frexp and std::ldexp #333

BenWibking opened this issue Jul 6, 2023 · 0 comments · Fixed by #471
Assignees
@BenWibking
Labels
bug Something isn't working floating-point issue with floating-point roundoff error priority:high high priority

Comments

@BenWibking
Copy link
Collaborator

BenWibking commented Jul 6, 2023 •
edited
Loading

The old implementation of the "not-quite-transcendental" functions in FastMath.hpp used magic numbers and reinterpret_cast to do the low-level bitwise manipulation.

Unfortunately, while it worked on all of our current platforms and compilers, the result of reinterpret_cast is undefined behavior, so it is better to use the C++ standard library functions std::frexp and std::ldexp to manipulate the mantissa and exponent. These are defined in a way that their results are independent of the floating point representation, but can be efficiently implemented with bitwise operations on systems that use IEEE floating-point numbers.

This is done in the new implementation of these functions in singularity-eos: https://github.com/lanl/singularity-eos/blob/2ea3957cc18cd4709b926f415dd68a8d4daf8821/singularity-eos/base/fast-math/logs.hpp#L38C3-L38C3
@BenWibking BenWibking added enhancement New feature or request floating-point issue with floating-point roundoff error labels Jul 6, 2023
@BenWibking BenWibking self-assigned this Jul 6, 2023
@BenWibking BenWibking changed the title update FastMath to use c++ standard library functions update FastMath to use std::frexp and std::ldexp Jul 6, 2023
@BenWibking BenWibking added priority:high high priority bug Something isn't working and removed enhancement New feature or request labels Dec 14, 2023
@BenWibking BenWibking mentioned this issue Dec 15, 2023
Merged
7 tasks
github-merge-queue bot pushed a commit that referenced this issue Dec 17, 2023
@BenWibking
use frexp and ldexp in FastMath (#471)
ed90651 
### Description
This avoids `reinterpret_cast` in FastMath.hpp, which is undefined
behavior. Instead, we use `ldexp` and `frexp` to manipulate the
floating-point mantissa and exponent for the fast approximate log
functions.

### Related issues
Fixes #333.

### Checklist
_Before this pull request can be reviewed, all of these tasks should be
completed. Denote completed tasks with an `x` inside the square brackets
`[ ]` in the Markdown source below:_
- [x] I have added a description (see above).
- [x] I have added a link to any related issues see (see above).
- [x] I have read the [Contributing
Guide](https://github.com/quokka-astro/quokka/blob/development/CONTRIBUTING.md).
- [x] I have added tests for any new physics that this PR adds to the
code.
- [x] I have tested this PR on my local computer and all tests pass.
- [x] I have manually triggered the GPU tests with the magic comment
`/azp run`.
- [x] I have requested a reviewer for this PR.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@BenWibking BenWibking

Labels
bug Something isn't working floating-point issue with floating-point roundoff error priority:high high priority
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

use frexp and ldexp in FastMath quokka-astro/quokka
1 participant
@BenWibking