Skip to content

[Tests] Add per-item deterministic RNG seeding for pytest - #1645

Open
merkelmarrow wants to merge 7 commits into
Xilinx:devfrom
merkelmarrow:fix/test-rng-determinism
Open

[Tests] Add per-item deterministic RNG seeding for pytest#1645
merkelmarrow wants to merge 7 commits into
Xilinx:devfrom
merkelmarrow:fix/test-rng-determinism

Conversation

@merkelmarrow

Copy link
Copy Markdown
Contributor

This change adds deterministic automatic seeding across all tests. Seeding is derived from each pytest node id which is individual to each test. This is done through an autouse fixture in tests/conftest.py.

# Example node id:
tests/fpgadataflow/test_foo.py::test_func_name[param_id]

This addresses problems we've historically had with determinism. This is a deliberate tradeoff; on the one hand we're losing the feedback that "one particular test fails 1% of the time, thus the reference output calculations aren't perfectly representative". But these kinds of failures add a lot of noise when we're more interested in finding actual regressions.

The intended RNG policy is described in the extension to the testing guidelines doc.

Implementation

  • 32 bit seed derived from sha256(nodeid) in tests/rng_seed.py. xdist loadgroup suffixes are stripped using the predefined regex in finn_ci.
  • tests/conftest.py adds an autouse fixture.
  • some tests in tests/util/test_rng_determinism.py cover suffix normalisation and numpy / torch seed behaviour
  • removed pinned seeds everywhere
  • refactored a test that relies on weights used across multiple loops to compute the weights upfront (instead of resetting the seed and computing the same weights on each loop iteration: test_fpgadataflow_convinputgenerator_rtl_dynamic.py)

Testing

  • a version of this change excluding minor changes was successful in Jenkins finn_ci_testing run 94.
  • individual tests files collect and run without problems in docker
  • new Jenkins run with latest changes

Other notes

Maintainers, note the guideline in the README that np.random.default_rng() should be seeded with finn_test_seed.

Tests that now fail due to tolerances at the deterministic seed should now be treated as deficient. Either the generator that produces the reference output should be improved to better model simulation outputs or the tolerances should be relaxed.

derive a stable per-test seed from each pytest node ID. Seed
Python, NumPy and the default CPU Torch generator before each
test, expose the resolved seed in JUnit output.

Signed-off-by: Marco Blackwell <mblackwe@amd.com>
Remove redundant seed-0 resets from independent fpgadataflow tests so
the autouse fixture gives each parametrisation reproducible distinct
data.

Generate dynamic-convolution weights once before the dimension loop
instead of resetting global NumPy state inside the model helper.

Signed-off-by: Marco Blackwell <mblackwe@amd.com>
Remove redundant seed resets from rewrite and conversion tests so each
parametrisation uses stable distinct data from the autouse fixture.

Signed-off-by: Marco Blackwell <mblackwe@amd.com>
Reomove the seed-0 reset from make_quant_test_model so scale and zeropt
uses the autouse per-item seed from conftest.

Signed-off-by: Marco Blackwell <mblackwe@amd.com>
Signed-off-by: Marco Blackwell <mblackwe@amd.com>
Add a randomness section to tests/README.md describing the per-item
seed fixture.

Signed-off-by: Marco Blackwell <mblackwe@amd.com>
Signed-off-by: Marco Blackwell <mblackwe@amd.com>
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.

1 participant