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.

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

Add reseed_rng option to p_iter_fork #39025

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from
Open

Conversation

mklss
Copy link

@mklss mklss commented Nov 23, 2024

This PR implements a convenience feature for the @parallel decorator, or more precisely, for p_iter_fork.
By setting reseed_rng=True the random number generator is reset in each subprocess by running set_random_seed(self.worker_seed).
It is ensured that the worker seeds are deterministically derived and thus, the results are always reproducible.

The new behaviour is useful for running probabilistic experiments and taking advantage of multiple cores.

The PR adds one "doc test". It would make sense to have additional (randomized) tests to assert that future modifications don't break the reproducibility. But I failed to locate a comprehensive test suite for sage/parallel, in particular, no existing tests for reproducibility of parallel computations. The sage/tests folder seems to contain other non-specific tests?

Coding style w.r.t. https://doc.sagemath.org/html/en/reference/misc/sage/misc/randstate.html

  • The documentation said with seed(worker_seed) should be used. I did not use it, because the current implementation reseeds the whole subprocess once and for all.
  • The documentation says NTL does not reproduce. This is a very surprising and unfortunate state of affairs. I expect that reseed_rng inherits this caveat. I did not make it explicit in the doc string, because if the problem occurs, then also reseed_rng=False should have it, but there is no mention of it.

📝 Checklist

  • The title is concise and informative.
  • The description explains in detail what this PR is about.
  • I have linked a relevant issue or discussion.
  • I have created tests covering the changes.
  • I have updated the documentation and checked the documentation preview.

The doc tests passed for me, but preview via running sage --docbuild tutorial html gives me ImportError: cannot import name count_all_local_good_types_normal_form' from 'sage.quadratic_forms.count_local_2' so doc tests don't work although that has probably nothing to with this PR.

Copy link

github-actions bot commented Nov 23, 2024

Documentation preview for this PR (built with commit 702cebf; changes) is ready! 🎉
This preview will update shortly after each push to this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant