test: consolidate the migrated suite — 885.6s -> 548.3s (-38%), zero flaky markers - #127
Merged
Conversation
Every _do_compute_orientations call was rebuilding the polar-geometry + template cache (~7 s) that build_matching_cache documents as geometry- and library-dependent only -- and every signal in the module shares one geometry. Build it once per library (module scope; class scope for the two-phase library) and pass it through the existing cache= kwarg, which is behaviour-preserving: the dict is exactly what each call would rebuild. The no-cache branch keeps in-suite coverage via the GUI om_run path (orientation_action.py passes no cache). All 4 pyxem reference positions in test_rows_and_quats_match_pyxem stay. 127.3 s -> 68.4 s locally.
_lib() (Phase.from_cif + res-10 library + build_matching_cache, ~5 s) was called per test in 8 of 9 tests. Module-scoped fixture; every consumer is read-only (match_correlations takes rot_mask as an argument, the render tests build fresh figures, nothing mutates sim or the cache). 48.3 s -> 20.9 s locally.
Each test spawned a fresh subprocess that paid interpreter + torch import +
cold CUDA init (~4-7 s) before its real work. Now ONE driver subprocess per
module runs all modes sequentially and prints one tagged
'RESULT_JSON <mode> {...}' line per mode; a module fixture parses them and
the tests keep their separate assertions. The driver still JSON-prints and
os._exit(0)s (Windows torch-CUDA teardown segfault), so partially-emitted
results survive a crash. A tiny F.linear primes cublasLt before any conv
work (Pascal cu124: first cublasLt init AFTER cuDNN convs fails with
CUBLAS_STATUS_NOT_INITIALIZED -- the multi-mode process is where that
ordering can arise). neural_detect's registry-mutating 'upgrade' mode runs
last. neural_batch_memory's torch-importing check_cuda probe subprocess is
replaced by the same in-process gpu_available() skipif
test_vector_orientation_gpu.py already uses.
69.6 s -> ~40 s locally (GPU box; the vog/neural legs are skipped on CI).
- vector_orientation_om: the overlay test re-ran the identical _make_vectors_tree + vom_generate_library flow of test_generate_then_run to read other properties of the same wizard (and did not force the CPU fit, so its background field fit could hit CUDA in-process); the no-library error test becomes the pre-generate stage of the same flow. - orientation_wizard: same shape -- the no-library om_run error check runs as the pre-generate stage of test_generate_refine_run. - orientation_overlay: the hook-registration test built the identical eager session + identical run_orientation(res=10); its assertions ride the attach/draw test. The lazy end-to-end test stays separate (different fixture + assertions). - post-_add_signal time.sleep(0.3-0.4)s replaced with conftest._settle. Assertion union throughout; nothing weakened. 67.2 s -> ~59 s locally.
…t, fold duplicate builds
- the two fixture time.sleep(0.8)s (ebsd_session / lazy_session) become
conftest._settle -- the comment ('let the selector debounce timers fire')
is literally _settle's use case.
- test_eager_data_still_gets_a_bounded_chunking rendered two 64x64-nav
scans (~9 s) to feed pure bytes/shape math; a 16x16 grid proves the same
three properties, with the budget shrunk to 1 MiB so both row counts stay
bytes-derived rather than ny-clamped.
- merges (assertion union, same _build + payload): PC-adoption +
caret-announce ride test_builds_a_wizard_with_a_resident_dictionary; the
ebsd_match stream assert rides the band-draw call that produces it;
hide/show rides the different-position overlay flow; the two-grain colour
check rides test_indexes_the_scan_into_an_ipf_window's identical run.
Kept minimal around the file top / device handling (PR #125 touches those
regions). File now 22.8 s locally.
- ebsd_indexing: the exact-euler dictionary was rebuilt by ~6 tests and the 10/12/15-deg dictionaries twice each -- module-scoped memos (pure data; remove_background/dictionary_index return new arrays). Two same-input merges: grain-2 consistency rides the two-grains disjointness result; the top-k tiling check rides test_tiled_equals_untiled at keep=5 (winner equality AND full ranked score lists on one whole/tiny pair). The 14.8 s finer-dictionary test is deliberately untouched (its 0.9 bound is calibrated to the 5-degree sampling). - ipf_3d: the stride-down ceiling is input-size-relative, so max_points=1000 against the 200x200 map exercises the same cap branch the 1200x1200 map did without 1.44M random quats through orix symmetry reduction (~9 s).
91 beam directions instead of 300; the generating template (index 40) is still a library member and the test already tolerates a symmetry-equivalent coarse pick -- the asserts are residual < 0.02 + the strain cap, both of which hold (residual actually improves, 0.0031 -> 0.0011). Measured honestly: the saving is ~1-2 s, not the projected ~10 s -- stage timing shows the test's cost is build_template_library (12.5 s, invariant to library size), not generate_library_from_phases (0.8 -> 0.4 s) or the fit (4.3 -> 3.8 s).
…olls The dominant pattern was a flat time.sleep(0.2-1.0) after _add_signal / a toolbar dispatch / a wizard call, waiting for the selector debounce timers and the first paint -- literally the condition conftest._settle polls (0.13 s floor + 5 ms poll, 3 s ceiling). 79 such sites across 28 files are now _settle(session), plus: - tutorial_data: the local flat-0.5 s _settle() shadowing conftest's is gone (23 sites); post-close waits poll the tree count instead (a full teardown leaves no plots for _settle's condition); the dispatch-by-name test rides the idempotency test's identical first load. - find_vectors_port: the render/overlay parity test re-ran the byte-identical Find-Vectors dispatch of the attach test; its assertions now ride the same computed result. - movie_block: the 0.8 s play window polls for >=2 movie_frame events with early exit; playback's loop test polls for an actual wrap (fires > n). - nav_async_cancel: the two flat 0.6 s waits become a barrier task on the 1-worker pool -- when it resolves, the reads AND their done-callbacks have provably finished. - console_preview: the three nav-rerun checks are negative-evidence windows (the rerun that must NOT happen has no event to poll for) -- kept as sleeps, shrunk 0.6 -> 0.3 s. Untouched by design: poll intervals, fire_delay/decode test doubles, race-geometry sleeps (navigator_race), wall-clock playback cadences, and negative-evidence windows (lifecycle, wizard_double_fire, test_hold, overlay_layers' painter-thread wait at :73 which has no clean observable). All 317 tests in the touched files pass (113 s for the set).
paint_signal_plots swallowed a failed set_data at DEBUG while the caller's attempt counter incremented around the call -- so on the flaky CI runs the counters asserted fine and the pixels were stale. It now RETURNS the number of successful paints (additive; callers that ignore it are unaffected), and the live preview records it as frames_landed. The two rerun-marked tests (test_block_paints_a_sample_frame, TestFindVectorsWiring::test_batch_attaches_a_preview_and_feeds_it) assert on that recorded success count instead of inferring it from pixels a concurrent repaint can race; their flaky/rerun markers are removed -- no @pytest.mark.flaky remains in the suite. test_nav_progressive's non-strict xfail failed DETERMINISTICALLY (its 40x0.05 s poll timed out every run, ~2.5 s/run forever): the fixture gated EVERY block, so every DP read -- including the load-time read at the crosshair's DEFAULT CENTER, which wedged the serial dispatcher before the probe ever ran -- blocked at the same gate as the navigator sum. The fixture now frees exactly ONE 8x8 nav-chunk (identified by frame CONTENT, not block_info coordinates -- dask's slicing pushdown rewrites those), the chunk holding both the default center and the probe position; the sum stays blocked on the other three chunks and the test is a real, passing assertion (3x stable locally). Waiting on the _NAV_COMPUTED event replaces the flat 0.2 s sleep (the fill defers until first signal paint).
…top-id check - report_orientation_embed: _om() rebuilt the (4,5) map (random-quat orix symmetry reduction) and pack_orientation re-packed it in nearly every test; both are memoised at module level for the default shape -- consumers only read them. The cap/stride tests keep their own pack_orientation call (they monkeypatch the module constants), and the swapped-identity cache test now builds two genuinely distinct maps (the memo's single identity is the opposite of what it asserts). - window_computing_overlay: the stop-carries-start-id check ran an identical session + _lazy_4d fill to read a second property of the same captured message list; it rides the bracket test.
Measured on the full green post-consolidation run (548.3s wall, summed 527.7s vs 811.5s before). Tests added by the parallel EBSD/nav-race PRs are absent and split at average cost, which pytest-split degrades gracefully.
…ailable
The consolidation swapped the CUDA-probe subprocess for the in-process
gpu_available() predicate, which is True on Apple MPS too -- so the
macos-latest leg RAN a test whose driver measures torch.cuda peak bytes and
asserts the model landed on cuda ("model did not load onto cuda: mps").
An in-process torch.cuda.is_available() check keeps the ~4 s probe saving
and restores the exact pre-consolidation skip semantics.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implements the approved Step-0b consolidation manifest. Full suite after: 2621 passed, 25 skipped, 0 failed, 0 xfailed — 548.3s wall (baseline 2634/25/1 xfail at 885.6s). The 14-test count delta is assertion-union merges; +1 is a former dead xfail that is now a real passing test. Shard-balance durations refreshed for CI.
Where the time went (top items):
cache=kwarg (the manifest's key finding:build_matching_cachewas rebuilt at 6.9-7.4s inside every compute call vs 0.02s injected):test_orientation_compute.py127.3 -> 68.4s.ipf_refinelibrary/module fixture: 48.3 -> 20.9s. CUDA subprocess consolidation 3->1 and 5->1 (tagged JSON result lines,os._exit(0)+ cuBLASLt priming kept): -30s. Wizard/overlay flow merges: -25s._settle()poll or event waits: ~40s. Genuinely-required debounce/wall-clock waits kept, each with the reason.build_template_library(12.5s) is invariant to library size; kept anyway (fit residual improves 0.0031->0.0011), invariant cost flagged as a future item.test_rows_and_quats_match_pyxem.Deflake (no rerun markers remain in the suite):
paint_signal_plotsnow returns the count of successfulset_datacalls (additive; failures still swallowed + DEBUG-logged — a progressive per-chunk callback must never fail the compute).live_signal.pyrecords it asframes_landed; the two formerly@pytest.mark.flakytests assert on that count deterministically.test_nav_progressive.pyis now a real passing assertion: the fixture gated EVERY block, so the load-time DP read at the default crosshair wedged the serial dispatcher before the probe ran. The fixture now frees exactly one 8x8 nav-chunk — identified by frame CONTENT, because dask's slicing pushdown rewritesblock_infocoordinates.Deliberately untouched (flagged in the manifest, no maintainer verdict yet): the multiphase resolution shrink (~6s) and the 5-degree dictionary coarsening. Say the word and they follow.
Merged with current main (EBSD lock + nav-race fix); merge-seam files re-verified (77 passed).
New top-10 slowest: 54.0s rows_and_quats (4 refs kept by decision) · 15.0 gpu strain subprocess · 14.6 finer-dictionary (deliberately capped) · 14.2 Ag integration (build_template_library-bound) · 11.5 vom flow · 11.2 multiphase (undecided flag) · 10.8/9.4 orientation e2e · 9.2 ipf_refine · 8.9 phase_recovered.