cache models inbetween test runs - #63
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR improves CI performance by caching BirdNET’s downloaded model/label assets between GitHub Actions runs, and ensuring the model cache directory used in CI is propagated into tox/pytest subprocesses.
Changes:
- Configure tox to forward the
BIRDNET_APP_DATAenvironment variable into test environments. - Set
BIRDNET_APP_DATAin CI to a stable, cacheable directory outside the checkout. - Add GitHub Actions caching for the model download directory and switch to
setup-uvwith caching enabled.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| pyproject.toml | Updates tox configuration to pass the model cache env var into test envs. |
| .github/workflows/ci.yml | Adds CI env + cache steps to persist downloaded model/label assets across runs and uses setup-uv caching. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
The inference pipeline used to create its processes and primitives with the global default start method, which is "fork" on Linux; forking after TensorFlow's multi-threaded runtime is loaded can deadlock the child. The pipeline now resolves its own start method (BIRDNET_START_METHOD env var > explicitly fixed global > "spawn") and creates every Process, Queue, Event, Semaphore, Lock and Value from that context. Parent-side reads of the global method (worker lazy-init, logging mixin, CoW preload gate, benchmark report) now use the session's effective method instead, so parent and children always agree. fork/forkserver remain supported via explicit opt-in. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The xdist "node down: Not properly terminated" worker crashes (geo pb tests on macOS, acoustic v3 onnx on py3.14) were pytest-timeout kills, not native crashes: in every occurrence the worker died exactly 300s after the test started. Those tests take ~140-250s on an idle dev machine, so contended CI runners push them over the limit; the thread timeout method then kills the whole worker process, which xdist reports as a crash. 600s keeps a real hang backstop while giving slow-but-healthy tests contention headroom. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
RingBufferResources._create gained a required start_method parameter, and the env-var resolver test assumed a non-spawn method exists, which is not true on Windows (spawn-only). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
40 was calibrated against the healthy single-env ceiling (~33-35 min), but ubuntu 3.12 runs py312 + py312-repro + py312-coverage and a single fork-lane deadlock (600 s per-test budget) pushed it over the cap. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The repro phase wedged on every macOS run: geo pb predicts never returned from TensorFlow's native eager execution (worker stack captured via faulthandler: quick_execute stuck indefinitely, macOS arm64 only, triggered under parallel suite load). tensorflow==2.20.0 is the culprit; 2.21.0 with otherwise identical pins does not hang (verified over full local repro-suite runs: 2/2 hangs on 2.20, 0 on 2.21). Only h5py moves with it. The watchdog (conftest.py, opt-in via BIRDNET_TEST_WATCHDOG_DIR, enabled in CI) covers the two failure modes pytest-timeout structurally cannot: a test wedging native code while holding the GIL (the timeout handler needs the GIL to run), and an xdist worker dying from a hard native crash, which can leave the controller waiting on the dead node forever with zero output (observed: a fork-lane worker crashed and the controller sat on the zombie for hours). faulthandler's C-level watchdog dumps all thread stacks and hard-exits without the GIL; the controller re-arms on forwarded test events so it dies at the idle deadline when those stop. faulthandler.enable() captures hard crash stacks in the same per-process dump files, which CI uploads as artifacts on failure. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
upload-artifact v4+ excludes hidden paths by default, so the .watchdog-dumps stack traces were silently dropped from failing jobs; include-hidden-files restores them. The download test's skip guard compared the full two-line error message against a one-line string and could never match; it now skips on any 5xx status and re-raises other errors instead of falling through to a misleading assert. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The fork-lane workers die by SIGKILL before faulthandler or the hang watchdog can react, which points at the OOM killer; dmesg on failure turns that hypothesis into direct log evidence. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
dmesg refuted the OOM hypothesis: the fork-lane worker is killed by pytest-timeout at the per-test timeout when a forked child wedges, but under xdist the timeout report dies with the worker and the orphaned child keeps the execnet socket open, leaving the controller blind until its idle deadline. -n 0 removes the worker: the timeout traceback lands in the job log and the lane ends at 600 s instead of 960 s. The dumps also exposed a latent watchdog bug: the xdist controller never collects, so its nodeid->timeout map is empty and every logstart armed the 900 s idle default -- less than a legitimate cold-cache load_model download, so a healthy download phase could be killed. The controller's fallback now covers the longest legitimate test; processes that collect themselves keep the tight idle fallback. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Every existing fork-marked test exercises a TensorFlow-based backend (PB or TFLite), all of which wedge on the fork-after-TensorFlow deadlock and are the reason the library defaults to spawn. Nothing proved that fork is actually usable for the non-TF backends the project is migrating toward (PyTorch, ONNX). This adds the missing coverage: the v3.0 pt and onnx backends run two parallel forked sessions and must agree, mirroring the perch/v2.4 pattern. macOS is skipped on the fork variant to match those tests (the Manager-helper fork hang there is backend-independent); Linux CI -- where birdnet forks in production -- is where the guarantee is enforced. The library needs no change: the start method resolver is already backend-agnostic, so fork is available to any backend via opt-in; this locks in that it *works* for everything except TensorFlow. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… them The pt/onnx fork tests landed in the same serial -n 0 fork lane as the TF fork tests. Because a fork-after-TensorFlow wedge kills the whole -n 0 process (that is why the lane is -n 0), and perch_v2 sorts before v3_0, the intermittent TF wedge killed the lane before the pt/onnx fork tests ran -- so the very tests meant to prove fork works without TensorFlow never executed (observed in run 30630301430: fork[pt]/fork[onnx] never ran; forkserver/spawn passed in the general lane). Give the deadlock-free non-TF fork tests a "fork_nontf" marker and run them first in their own process, ahead of the TF fork tests. A wedge in the TF lane can no longer stop them from recording a result. They keep the "fork" marker too, so the parallel general lane still excludes them (forking after TF is loaded there would deadlock). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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
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.
No description provided.