Repository navigation
Conversation
Remove --randomly-dont-reset-seed from the CI pytest invocation so that pytest-randomly reseeds before each test from the session seed and the test's node id. Previously the RNG state carried over between tests in a CI job, so adding or removing a test changed the random weights of the tests after it. Fix the pytest_randomly.random_seeder entry point in pyproject.toml. The unquoted dotted key registered the group as "pytest_randomly", so hls4ml.reseed was never called and TensorFlow and PyTorch were not seeded.
| 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" |
There was a problem hiding this comment.
With the entry point fixed, hls4ml.reseed runs for the first time: once in the report header and again at setup, call and teardown of every test. That brings up two things in hls4ml/__init__.py:
- It imports frameworks unconditionally and only catches
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. - Every job now loads both TensorFlow and torch. The
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 reseed only seeds frameworks that are already imported, since there's nothing to seed in a framework nobody has loaded yet:
import sys
def reseed(newseed):
if 'tensorflow' in sys.modules:
sys.modules['tensorflow'].random.set_seed(newseed)
if 'torch' in sys.modules:
sys.modules['torch'].manual_seed(newseed)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.
| 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" |
There was a problem hiding this comment.
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.
hls4ml.reseed now runs at every pytest-randomly reseed point. Importing TensorFlow and PyTorch from it loaded both frameworks into every test job, and any import failure other than ImportError aborted the session. Seed only the frameworks already in sys.modules instead. Add a test that the pytest_randomly.random_seeder entry point is registered, since its breakage in #1094 went unnoticed.
|
PR #1604 should fix the sphinx error. |
Description
Makes the random state in the pytests repeatable per test, so that adding or removing a test no longer
changes the random weights other tests see. This was making synthesis resource numbers move by more than
10% for unrelated changes, and loosening the tolerance would hide real changes.
Two problems, one line each:
CI turned off per-test reseeding.
test/pytest/ci-template.ymlpassed--randomly-dont-reset-seed.With it, pytest-randomly seeds once at session start and the RNG stream runs on through every test in the
job. CI jobs group several test files, so any test earlier in a job changed the state of all later ones.
Without the flag, pytest-randomly reseeds
random,numpy.randomand all registered seeders before eachtest with
seed + crc32(nodeid), which depends only on the session seed (42) and the test's own id.The TF/PyTorch seeder was never registered. Since the move to
pyproject.toml(Lazy converter imports and migrate to pyproject.toml #1094) the entrypoint was written as
entry-points.pytest_randomly.random_seeder = "hls4ml:reseed". TOML splits unquoteddotted keys, so this registered group
pytest_randomly, namerandom_seeder, while pytest-randomly looksin group
pytest_randomly.random_seeder.hls4ml.reseedwas therefore never called andtensorflow.random.set_seed/torch.manual_seednever ran. Quoting the group restores what the originalsetup.cfgdeclared:Expect synthesis resource baselines to shift once, since every test now gets a different seed than before;
they should be stable afterwards. Module- or session-scoped fixtures that build random models still take the
seed of the first test that uses them, so they can change if a test is added in front of them or
-kis used.Existing editable installs keep the old, broken entry point in their metadata until reinstalled
(
pip install -e . --no-deps). CI does a fresh install and is not affected.Type of change
io_type, Strategy, or config attributeAffected areas
Backends: n/a (test infrastructure only)
hls4ml.model)Frontends:
Components:
hls4ml/templates/Configurations affected and exercised
io_typeImpact on generated HLS
Numerical behavior: no change to hls4ml itself. Tests that relied on unseeded random weights will see
different (but now repeatable) weights.
No synthesis was run for this PR.
Tests
Added test_reseed.py to the the reseeding, since it had been broken and unnoticed for a while.
Verified with throwaway tests (not committed) that hash newly initialized weights and numpy draws. Each was
run twice on its own and once after another random-consuming test, with
--randomly-seed=42 --randomly-dont-reorganize:--randomly-dont-reset-seedBefore the entry-point fix, torch weights differed even between two identical runs.
Existing tests still pass with the new flags:
pytest test_keras_api.py -k "Vivado and io_parallel" --randomly-seed=42 --randomly-dont-reorganize(Keras 2.14): 14 passed
pytest test_pytorch_api.py -k "Vivado and io_parallel" --randomly-seed=42 --randomly-dont-reorganize(PyTorch 2.10): 29 passed
Test configuration (OS, Python, ML framework version, HLS tool version): macOS (Darwin 25.6), Python 3.11
(Keras 2 env) and 3.12 (Keras 3 and PyTorch envs), TF 2.14 / Keras 2.14, Keras 3.13, PyTorch 2.9.1 and 2.10,
pytest-randomly 4.0.1 and 4.1.0. No HLS tools used.
AI assistance disclosure
Tool(s) and model(s): Claude Code (Claude Opus 5.5)
Where it was used: diagnosis, both one-line changes (
pyproject.toml,test/pytest/ci-template.yml), theverification runs and this description.
If anything other than None is ticked, confirm all of the following:
it. I have read and understood every line and can explain and defend it
in review.
no invented APIs, config keys, pragmas or citations.
actually performed, not from agent output.
my knowledge it does not reproduce third-party code that would conflict
with it.
Checklist
Required:
pre-commiton the files I edited.test/pytestcovering this change (a bug fix shouldadd a test that fails on
mainand passes here).commented-out blocks or unrelated changes.
If applicable:
docs/updated.Release note
🤖 Generated with Claude Code