Skip to content

Implement new RNG scheme - #234

Merged
mosesyhc merged 33 commits into
mainfrom
188RNGs
Jul 16, 2026
Merged

Implement new RNG scheme#234
mosesyhc merged 33 commits into
mainfrom
188RNGs

Conversation

@jared321

@jared321 jared321 commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Tentative PR Self-review

  • Review all changes made here
  • Confirm test script fails with invalid bit generator name
  • Confirm that new MH benchmark is acceptable
    • New benchmark is deterministic as expected
  • Confirm sampler test script passing with no changes except for one altered test.
  • Confirm that ball drop notebook is now deterministic when run locally
    • Altered seed to see difference pop up. Reset seed to see no meaningful differences
  • Review new/updated documentation as rendered by RTD for this PR
  • Review contents of both notebooks as rendered locally
  • Confirm all actions passing

jared321 added 5 commits July 8, 2026 17:52
The docs are rough/wrong and the code isn't using any of this yet.
Ran the ball drop notebook to see it fail with correct error message since a
scipy.stats RNG was not being set into surmise yet.  Passing once I added in the
set_RNG call.

The pytest tests are failing because they haven't set the RNG either.  I'll let
Moses do that.
@jared321 jared321 self-assigned this Jul 10, 2026
@jared321

jared321 commented Jul 10, 2026

Copy link
Copy Markdown
Contributor Author

@mosesyhc Can you please propagate the use of the new global surmise RNG throughout the code base. Please see the changes made so far to see what I have already done (nbdime, which is installed by the tox book task, might be useful).

  • The pytests are failing because they aren't setting an RNG in surmise
  • Need to convert all random variable draws (aside from in the samplers) to use this global RNG
  • Iterate on the RNG content is User Guide --- example code needs to be redone
  • Iterate on the RNG content in the Dev Guide

jared321 and others added 21 commits July 10, 2026 12:34
(1) Ensure that no surmise RNG is used except within test functions
(2) Reduce redundancy in calling shared variables from shared_scenario.py
(3) Allow for fixtures to be called by test to ensure "no_rng" (RNG cleared), "seeded_RNG" (surmise RNG set properly)
@mosesyhc

mosesyhc commented Jul 15, 2026

Copy link
Copy Markdown
Member

A Jupyterbook page is added to illustrate the use of surmise RNG and reproducible results. Guides and README are modified to include a link to the Jupyterbook (minimally here, the JB reference should be made explicit and prominent in the release.)

@jared321 ready for your review. (realizing that this will be a self-review because you initiated the PR)

jared321 and others added 6 commits July 15, 2026 13:56
As expected, the new results were not identical to the benchmark.  However, the
new results look reasonable.
Random draws not using the global RNG.  Internal tests probably shouldn't
override the underlying failure message, but rather only print more information
to help understand the context of the failure.

This should hopefully get actions passing again.
@jared321
jared321 requested a review from mosesyhc July 15, 2026 23:33

@mosesyhc mosesyhc 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.

Reviewed via

  • the running of Jupyterbook
  • manually clear RNG and confirm failure
  • set same RNG seeds to confirm reproducibility
  • set different RNG seeds to confirm that different samples are drawn

@mosesyhc
mosesyhc merged commit 3707d7e into main Jul 16, 2026
55 of 56 checks passed
@jared321
jared321 deleted the 188RNGs branch July 17, 2026 15:45
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.

2 participants