Repository navigation
Make pytest random seeds repeatable per test #1601
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -88,7 +88,7 @@ optional-dependencies.testing-qkeras-v3 = [ | |
| optional-dependencies.xls = [ "xls-python>=0.1.9875" ] | ||
| urls.Homepage = "https://fastmachinelearning.org/hls4ml" | ||
| scripts.hls4ml = "hls4ml.cli:main" | ||
| entry-points.pytest_randomly.random_seeder = "hls4ml:reseed" | ||
| entry-points."pytest_randomly.random_seeder".hls4ml = "hls4ml:reseed" | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could a small test guard this registration? It has been silently broken since #1094, and a future edit or formatter rewrite of this key could break it again without any test failing. Something like: from importlib.metadata import entry_points
def test_reseed_entry_point_registered():
eps = entry_points(group='pytest_randomly.random_seeder')
assert any(ep.value == 'hls4ml:reseed' for ep in eps)Because this reads the installed metadata, it will also fail in an editable install made before this change, which is a useful hint to reinstall.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added. |
||
|
|
||
| [tool.setuptools] | ||
| packages = [ "hls4ml" ] | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| from importlib.metadata import entry_points | ||
|
|
||
|
|
||
| def test_reseed_entry_point_registered(): | ||
| eps = entry_points(group='pytest_randomly.random_seeder') | ||
| assert any(ep.value == 'hls4ml:reseed' for ep in eps) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With the entry point fixed,
hls4ml.reseedruns for the first time: once in the report header and again at setup, call and teardown of every test. That brings up two things inhls4ml/__init__.py:ImportError. Ifimport torchraisesOSError(for example missing CUDA libraries), orimport tensorflowfails with something else (a protobuf or numpy ABI mismatch), the whole pytest session errors out at the header. That includes jobs that never use either framework.KERAS_BACKEND=torch/jax jobs (e.g.test_pquant_pytorch) import TF, and the TF-only jobs import torch. That costs startup time and memory, and it puts both frameworks in one process where they weren't before.Both go away if
reseedonly seeds frameworks that are already imported, since there's nothing to seed in a framework nobody has loaded yet:One caveat: a framework imported for the first time inside a test (rather than at module level) would skip that test's setup reseed, but it would still be seeded from the next reseed point on.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done