common/ompio: possible rounding issue #6287
Conversation
Similar to open-mpi#6286 rounding number of bytes into a single precision floating point value to round up the result of a division is a potential risk due to rounding errors. - remove floating point operations for `round up` - removes floating point conversion for round down (native behavior of integer division) Signed-off-by: René Widera <[email protected]>
|
Can one of the admins verify this patch? |
|
To avoid style changes I keeped the indention for this PR equal to the lines I changed even if there are tabs in the first column. This will not follow the contribution guide lines but I tried to avoid to change other code around the touched lines. |
| int size_smallest_group = 0; | ||
| int num_groups = 0; | ||
| int ret = OMPI_SUCCESS; | ||
| OMPI_MPI_COUNT_TYPE bytes_per_agg_group = 0; |
There was a problem hiding this comment.
Is OMPI_MPI_COUNT_TYPE the right type for a byte count here or should it be MPI_Aint?
|
ok to test |
|
I am honestly still unclear why the ceil and floor functions are considered inadequate, i.e. not convinced this is truly necessary. |
|
@edgargabriel Since |
|
after thinking a bit more about it, the problem pinpointed by this fix is a legitimate concern. Whether we use a double precision value and the ceil function for resolving it, or your approach doesn't really matter in my opinion, so we can this solution as well. |
4 participants
Similar to #6286 rounding number of bytes into a single precision floating point value to round up the result of a division is a potential risk due to rounding errors.
round upSigned-off-by: René Widera [email protected]