Fix pre-existing CI failures on macOS Intel and model download tests - #58
Conversation
The load_model tests were not failing because of interrupted downloads but because of the global 300s pytest timeout: the v3.0 models are ~520 MiB and Zenodo serves them at ~2 MiB/s, so they need 5-6 min. The progress bars in CI show them being killed at 84-98% while still downloading at full speed. Give load_model tests 1800s and leave the 300s guard in place everywhere else. Because tox stops at the first failing command, these timeouts also hid the rest of the suite, which masked two real failures on macOS Intel: - start_time/end_time are float32, but on numpy 1.x the intermediate products stay float32 as well (numpy 2 widens them to float64 via NEP 50), so speeds that are not exactly representable drift by ~1 ULP. The tests asserted the assert_allclose default rtol of 1e-7, which is below float32 eps (~1.19e-7). macOS Intel is the only runner pinned to numpy 1.26 (via tensorflow <2.17). - torch 2.2.2 is the last release with x86 macOS wheels and is too old for the v3.0 TorchScript model. The model loads fine, but the inference workers die and the session is cancelled, so skip the v3.0 torch tests there. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR addresses pre-existing CI instability by preventing large model downloads from being killed by the global pytest timeout, stabilizing float32 time assertions across NumPy versions, and skipping v3.0 Torch backend inference tests on macOS Intel where the available Torch wheel is too old.
Changes:
- Add a
pytest_collection_modifyitemshook to apply a longer timeout to tests markedload_model. - Loosen
assert_allclosetolerance for float32-based time calculations in encoding structured-array tests. - Introduce and apply an Intel-macOS skip helper for v3.0 Torch backend inference tests.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/birdnet_tests/helper.py | Adds ensure_v3_0_torch_backend_or_skip() to skip v3.0 Torch inference on Intel macOS. |
| src/birdnet_tests/conftest.py | Applies a per-test timeout override (1800s) for tests marked load_model. |
| src/birdnet_tests/acoustic_models/v3_0/model_py/test_predict/test_acoustic_predict_model_v3_0.py | Uses the new skip helper for v3.0 Torch backend predict-session tests. |
| src/birdnet_tests/acoustic_models/v3_0/model_py/test_encode/test_acoustic_encode_model_v3_0.py | Uses the new skip helper for v3.0 Torch backend encode-session tests. |
| src/birdnet_tests/acoustic_models/inference/encoding/encoding_result_py/test_encoding_to_structured_array.py | Adjusts float32 time comparisons to tolerate 1-ULP drift across NumPy promotion behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Some tests deadlock in a C-level multiprocessing wait. pytest-timeout's default signal method cannot interrupt a thread blocked in a C call, so those hangs ran until the 360-minute job limit killed them - burning ~24 runner-hours per hang and producing no diagnostic. Switch to the thread method, which kills the process and dumps stacks, turning such hangs into a fast failure with a traceback. Windows already used this method (it has no SIGALRM) and never exhibited the hang. Also drop timeout-minutes from 360 to 60. The slowest passing job is ~30 min, so this is only a backstop against a wedged run; pytest-timeout bounds individual tests. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Follow-up: the "never-finishing" jobs are a separate deadlock, now boundedThe first run of this PR fixed macOS Intel and the download timeouts (both
Root cause of the hangsFrom The signal method runs its handler between Python bytecodes, so it cannot interrupt a thread blocked in a C-level This commit
This turns the 6-hour silent hangs into fast, diagnosable failures. The underlying deadlock itself (the library's process manager inherits a global start method that a prior |
A push to a PR did not cancel the previous run, so a wedged run kept burning runner hours until manually stopped. Add a concurrency group that cancels the in-progress run for the same PR ref; pushes to main still run to completion. The fork/TensorFlow deadlock hangs in native code while holding the GIL, which pytest-timeout cannot interrupt in either the signal or thread method, so the job-level limit is the only thing that stops it. Tighten it from 60 to 50 min: the slowest healthy job is ~32 min, so this keeps headroom while bounding the cost of a hang. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Run 2 analysis + decision: harden CI, leave library runtime as-isThe thread-method run finished 7 passed, 6 cancelled, 1 failed. Breakdown:
The thread-method change did not stop the hangs (fired 0 times)Important correction to my earlier claim. My local proof deadlocked a
pytest-timeout cannot rescue this class of hang in any mode. Windows never hangs because it uses spawn, not fork. DecisionPer maintainer: harden CI only, leave the library runtime unchanged, treat the deadlock as known-flaky. The root fix (pinning the process manager to a spawn/forkserver context instead of inheriting the global default) is a runtime behavior change and is deferred. This commit:
|
Fixes three pre-existing failures on
main. None of them originate from #54, but they are what is turning that PR's CI red.The
load_model"download flakes" are the 300s timeoutThey are not interrupted downloads, so retries would not have helped. The v3.0 models are ~520 MiB and Zenodo serves them at ~2 MiB/s, so a single download needs ~5-6 min against a global
timeout = 300. The tqdm progress bars in run 29565314026 show them being killed mid-download at full speed:This also explains why only the v3.0 models (~516 MiB) ever fail while the 13-73 MiB v2.4/geo models never do. The
Read on closed or unwrapped SSL socketin the tracebacks is just pytest-timeout interrupting the read, not the cause.load_modeltests now get 1800s via apytest_collection_modifyitemshook. Every other test keeps the 300s guard, so real hangs are still caught.Two real failures that were hidden behind it
Because tox stops at the first failing command, a timeout in the
load_modelrun meant the rest of the suite never executed. The macos-15-intel 3.12 job onmaindid get past it, and shows both of these already present.Float32 time tolerance. Not an x86-vs-ARM issue: it is numpy 1.26 vs 2.x promotion.
start_time/end_timeare float32, and on numpy 1.x the intermediate products stay float32 too, while numpy 2 widens them to float64 via NEP 50. Speeds that are not exactly representable therefore drift ~1 ULP. The tests assertedassert_allclose's defaultrtol=1e-7, which is below float32 eps (~1.19e-7), so the assertion was never sound on float32 data. macOS Intel is the only runner pinned to numpy 1.26 (viatensorflow <2.17). Nowrtol=1e-6;start_timewas at 7.5e-8 and one perturbation from flaking, so both are loosened.v3.0 torch backend on macOS Intel.
pt = ["torch >= 2.0.0"]has no upper pin, so macOS Intel resolves torch 2.2.2 (the last x86 macOS wheel), which is too old for the v3.0 TorchScript model.torch.jit.loadruns inside the inference worker rather than at model-load time, which is whytest_v3_0_ptpasses while every v3.0 pt inference test fails withRuntimeError: Analysis was cancelled. The onnx backend works on the same machines. Skipped via a newensure_v3_0_torch_backend_or_skip(), matching the existingtensorflow <2.17pin pattern for that platform.Not addressed here
test_tflite_fp32_twice_two_sessions_parallel_processes_forkhangs nondeterministically (it timed out on ubuntu-24.04 / 3.12, passed on 3.11 and 3.13). It is the documented fork-in-a-multi-threaded-process hazard, matching theQueueFeederThreadstacks stuck inwaiter.acquire()- the same root cause as the existingensure_not_mac_or_skip() # reason unknownline.Skipping it on Linux would make it dead code:
use_fork_or_skip()only proceeds on Linux/macOS and macOS is already skipped. That is a delete-or-redesign decision, left for a follow-up. Note it will likely surface more often now that the suite actually runs to completion.Verification
load_modeltests and 300s for everything else, checked through the plugin's own API.1.0596381286823678e-07, matching the CI log's1.05963813e-07; it fails atrtol=1e-7and passes at1e-6.main.🤖 Generated with Claude Code