Skip to content

Restore determinism in beersheba tests - #977

Closed
gonzaponte wants to merge 6 commits into
next-exp:masterfrom
gonzaponte:deterministic-b7-tests
Closed

gonzaponte wants to merge 6 commits into
next-exp:masterfrom
gonzaponte:deterministic-b7-tests

Conversation

@gonzaponte

Copy link
Copy Markdown
Collaborator

This PR addresses instabilities in the RL algorithm caused by roundoff errors. At the same time, it fixes a silent issue by which the "joint" mode of beersheba was not being exercised. After fixing this issue, the exact_result tests are no longer flaky.

Closes #975.

@gonzaponte
gonzaponte requested a review from jwaiton October 3, 2026 15:16

@jwaiton jwaiton left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Just a minor nitpick, other than that good work!

Comment thread invisible_cities/conftest.py Outdated
The separate-mode fixture updated `deconv_params` on the shared
`beersheba_config` dictionary. Pytest reuses a fixture's returned object
within a test, so requesting both fixtures could leave the base
configuration set to separate mode as well.

A deep copy of the base configuration before changing to separate mode
prevents this issue. Tests can now use independent configurations for
joint and separate deconvolution, so each variant exercises the mode it
intends to check.
FFT convolution can produce tiny negative or near-zero values through
floating-point roundoff. Replacing only exact zeros (as we have been
doing so far) with machine epsilon leaves those values in place;
dividing the image by them can greatly amplify numerical noise during
the Richardson–Lucy update.

Clamping the convolved estimate to at least machine epsilon before
calculating the relative blur keeps the denominator positive and
prevents small roundoff differences from causing large changes in the
reconstruction.
@gonzaponte
gonzaponte force-pushed the deterministic-b7-tests branch from 86810fc to 4fd31f8 Compare October 7, 2026 09:13

@jwaiton jwaiton left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This PR refactors some of the beersheba tests, removing their flakiness, a longstanding issue in IC.

Good work!

gonzaponte pushed a commit to gonzaponte/IC that referenced this pull request Oct 8, 2026
next-exp#977

[author: gonzaponte]

This PR addresses instabilities in the RL algorithm caused by roundoff
errors. At the same time, it fixes a silent issue by which the "joint"
mode of beersheba was not being exercised. After fixing this issue, the
exact_result tests are no longer flaky.

[reviewer: jwaiton]

This PR refactors some of the beersheba tests, removing their flakiness,
a longstanding issue in IC.
@gonzaponte gonzaponte closed this Oct 8, 2026
@gonzaponte
gonzaponte deleted the deterministic-b7-tests branch October 8, 2026 13:33
@gonzaponte

Copy link
Copy Markdown
Collaborator Author

Oops, messed up the merge.

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.

Beersheba tests are not deterministic

2 participants