Skip to content

Avoid floating point comparison issues#950

Merged
mcidlaso merged 7 commits intonext-exp:masterfrom
gonzaponte:rounding-error
Jan 20, 2026
Merged

Avoid floating point comparison issues#950
mcidlaso merged 7 commits intonext-exp:masterfrom
gonzaponte:rounding-error

Conversation

@gonzaponte
Copy link
Collaborator

Some tests that check for an "exact result" have been problematic because they rely on floating point comparisons. This is machine-dependent, which means a test may pass locally, but fail on GHA or viceversa. With direct comparisons, this is usually addressed by adding a tolerance, which we already do. However there are other cases where this isn't enough

  • cumulative rounding errors: typical of beersheba, where the iterative process seems to accumulate errors beyond what one might expect using doubles (?)
  • discretization of continuous values: typical of paolina, where voxelization is sensible to tiny differences between floating point values, despite our attempts to avoid them.

In this PR, we bypass all these issues by rounding values to a safe number while maintaining enough precision.

Copy link
Member

@Luismiguelvillar Luismiguelvillar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks good to me. I just have a question regarding the chosen precision. Is there a specific reason for rounding to 6 decimal places?

deco_dst.E = np.round(deco_dst.E, 6)

@gonzaponte
Copy link
Collaborator Author

Not a fundamental reason, but a practical one. Rounding to more digits produced discrepancies between my computer and GHA. 6 digits seems enough to maintain precision while avoiding these issues.



@given(bunch_of_hits)
def test_round_hits_positions_in_place(hits):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we also test how the function behaves with edge cases such as NaNs, infinities, or empty hit lists?

@Luismiguelvillar
Copy link
Member

Not a fundamental reason, but a practical one. Rounding to more digits produced discrepancies between my computer and GHA. 6 digits seems enough to maintain precision while avoiding these issues.

Makes sense! But maybe it will be a good idea to let the user change this precision, similar to the xyz in hits?

Copy link
Member

@Luismiguelvillar Luismiguelvillar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Everything looks fine!

@gonzaponte
Copy link
Collaborator Author

@mcidlaso please merge

@mcidlaso mcidlaso merged commit f61e394 into next-exp:master Jan 20, 2026
1 check passed
@gonzaponte gonzaponte deleted the rounding-error branch January 20, 2026 17:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

Comments