diff --git a/.gitea/workflows/traceability-check.yml b/.gitea/workflows/traceability-check.yml index f71a8a6..6456fb6 100644 --- a/.gitea/workflows/traceability-check.yml +++ b/.gitea/workflows/traceability-check.yml @@ -7,7 +7,8 @@ name: Traceability Validation # prefixes count, which file suffixes are source, which directories to scan, # the threshold - lives in traceability.toml at the repo root, and the same # extractor is shared by all three JRay components. Copying this file into -# another component needs no edits. +# another component needs no edits - including the container image below, which +# is a stock public one and names nothing about this repo. # # NOTE: the runner here is an Intel N100 with no discrete GPU. This job is only # ever static analysis of source comments plus markdown parsing, so it is cheap; @@ -31,6 +32,20 @@ jobs: runs-on: linux/amd64 name: Check requirement traces + # Gitea's act_runner executes JS actions - actions/checkout and + # upload-artifact are both JS - with the `node` binary found INSIDE the job + # container, not one it supplies. The runner's default image has none, so + # without this block the job dies at "Cannot find: node in PATH" before the + # repository is even checked out, and every later step fails on a missing + # working tree rather than on anything it was meant to check. + # + # node:20-bookworm rather than a Python image because node is the part that + # cannot be worked around: bookworm's python3 is 3.11, which has tomllib and + # is therefore already everything the stdlib-only extractor needs. Debian 12 + # also carries the git the pull_request diff step below shells out to. + container: + image: node:20-bookworm + steps: - name: Checkout repository uses: actions/checkout@v4 diff --git a/.gitea/workflows/unit-tests.yml b/.gitea/workflows/unit-tests.yml index 665dfac..6c18792 100644 --- a/.gitea/workflows/unit-tests.yml +++ b/.gitea/workflows/unit-tests.yml @@ -40,7 +40,7 @@ jobs: # bumping the tag in scripts/ci/build_builder_image.sh AND here, in one # commit -- see that script's header. container: - image: gitea.tourolle.paris/dtourolle/sae-builder-cpu:v1 + image: gitea.tourolle.paris/dtourolle/sae-builder-cpu:v3 steps: - name: Checkout repository @@ -73,17 +73,27 @@ jobs: # line above and the image that actually landed disagree, which is # exactly the drift the pinning exists to prevent -- so it fails the # job rather than building against an unknown toolchain. - [ "$SAE_BUILDER_VERSION" = "v1" ] || { - echo "image reports version '$SAE_BUILDER_VERSION', workflow pins v1" >&2 + [ "$SAE_BUILDER_VERSION" = "v3" ] || { + echo "image reports version '$SAE_BUILDER_VERSION', workflow pins v3" >&2 exit 1 } + # Pinned to a version, never `latest`, for the same reason the builder + # image above is: a dump is an input to the tests, so a moving `latest` + # would let a re-upload retroactively change what an earlier green build + # proved. It also removes a credential from this job entirely -- package + # DOWNLOADS are anonymous while the repo is public, and only resolving + # `latest` needs a token (the list-packages endpoint requires auth on this + # instance). `latest` was the sole reason this step wanted GITEA_TOKEN, + # and no such secret is configured, so it could never have resolved. + # + # Bumping the fixtures means uploading a new version with + # scripts/artifacts/push_artifacts.sh replay-fixtures and editing the SHA + # here, in the same commit -- as with the image tag. - name: Fetch replay fixtures - env: - GITEA_TOKEN: ${{ secrets.GITEA_TOKEN }} # bash, not sh: the script declares #!/bin/bash and uses `set -o # pipefail` and arrays, which dash does not have. - run: bash scripts/artifacts/pull_artifacts.sh replay-fixtures latest + run: bash scripts/artifacts/pull_artifacts.sh replay-fixtures ff3b8eb # pull_artifacts.sh warns and continues when a package version is missing, # which is right for a developer pulling one artifact of several and wrong @@ -107,7 +117,9 @@ jobs: echo "Replay fixtures are absent, so the T2 tier cannot run." >&2 echo "They are not in git (tests/fixtures/dumps/.gitignore) -- they" >&2 echo "live in the Gitea generic package registry and are pulled by" >&2 - echo "the step above, which needs GITEA_TOKEN to resolve 'latest'." >&2 + echo "the step above, at the version pinned there. Check that the" >&2 + echo "version still exists in the registry: pull_artifacts.sh warns" >&2 + echo "and continues on a missing one rather than failing." >&2 exit 1 fi diff --git a/.gitmodules b/.gitmodules index c526030..49617a7 100644 --- a/.gitmodules +++ b/.gitmodules @@ -4,4 +4,4 @@ branch = master [submodule "jray-project"] path = scripts/vendor/jray-project - url = git@gitea.tourolle.paris:dtourolle/jray-project.git + url = https://gitea.tourolle.paris/dtourolle/jray-project.git diff --git a/Dockerfile.builder-cpu b/Dockerfile.builder-cpu index a81cc54..1203e46 100644 --- a/Dockerfile.builder-cpu +++ b/Dockerfile.builder-cpu @@ -92,6 +92,11 @@ RUN apt-get update && apt-get install -y --no-install-recommends \ git \ ca-certificates \ curl \ + # unzip is not optional here: the T2 replay fixtures ship as a zip in the + # generic package registry and scripts/artifacts/pull_artifacts.sh unpacks + # them with it. Without unzip that step exits 127 and the replay tier has no + # input -- which is a missing tool reported as a missing fixture. + unzip \ # Gitea's act_runner executes JS actions (actions/checkout, upload-artifact) # with the `node` found *inside* the container. Without this the job cannot # even check the repository out. Same reason as the kpnpp-builder image. @@ -116,6 +121,17 @@ RUN apt-get update && apt-get install -y --no-install-recommends \ # Both the main build (CMakeLists.txt:162) and the test target # (tests/CMakeLists.txt:42) discover it through pkg-config `openblas`. libopenblas-dev \ + # FFTW3. The learned scene-boundary detector is compiled into result_sink + # whenever SAE_SCENE_XGB is on -- which is the default -- and it pulls in + # xgboost + FFTW + ffmpeg together (CMakeLists.txt:377). The find_library is + # REQUIRED and runs at CONFIGURE time, so a missing libfftw3 fails the whole + # cmake step: the `sae_tests` target never gets as far as being irrelevant + # to it. This image predates the detector, which is why it was absent. + # + # Turning SAE_SCENE_XGB off in CI would also make the configure pass, and it + # is the wrong fix: CI is the only place these tests run, so the flag that + # silences a subsystem there silences it everywhere. + libfftw3-dev \ # Python: the build itself needs the interpreter and headers # (find_package(Python COMPONENTS Interpreter Development.Module) at # CMakeLists.txt:254, for the nanobind modules). numpy/h5py/scipy are for @@ -140,14 +156,18 @@ RUN apt-get update && apt-get install -y --no-install-recommends \ zlib1g-dev \ && rm -rf /var/lib/apt/lists/* -# Fail the image build, not the CI run, if OpenBLAS or swresample are not -# discoverable the way CMakeLists.txt discovers them. An image that ships +# Fail the image build, not the CI run, if OpenBLAS, swresample or FFTW are +# not discoverable the way CMakeLists.txt discovers them. An image that ships # libopenblas but no openblas.pc would compile the scalar fallback in silence. +# FFTW is checked as a bare .so rather than by pkg-config because that is how +# CMake looks for it -- find_library(FFTW3_LIB fftw3), not pkg_check_modules. RUN set -eux; \ pkg-config --exists openblas; \ echo "openblas $(pkg-config --modversion openblas)"; \ pkg-config --exists libswresample; \ - echo "swresample $(pkg-config --modversion libswresample)" + echo "swresample $(pkg-config --modversion libswresample)"; \ + ls /usr/lib/*/libfftw3.so > /dev/null; \ + echo "fftw3 $(pkg-config --modversion fftw3)" # ─── ONNX Runtime, CPU provider only ───────────────────────────────────────── # diff --git a/external/KPN b/external/KPN index 771b9f8..af9da7d 160000 --- a/external/KPN +++ b/external/KPN @@ -1 +1 @@ -Subproject commit 771b9f85938dbc1ac449ed4a730fa20ec298609e +Subproject commit af9da7db81cbc28e8f13e7cff683a0218a259881 diff --git a/scripts/ci/build_builder_image.sh b/scripts/ci/build_builder_image.sh index acce6ec..f051a10 100644 --- a/scripts/ci/build_builder_image.sh +++ b/scripts/ci/build_builder_image.sh @@ -5,9 +5,9 @@ # TRACES: DP-007 | PR-004 # # Usage: -# scripts/ci/build_builder_image.sh # build only, tag v1 +# scripts/ci/build_builder_image.sh # build only, tag v3 # scripts/ci/build_builder_image.sh --push # build and push -# scripts/ci/build_builder_image.sh --tag v2 --push # bump the pinned tag +# scripts/ci/build_builder_image.sh --tag v4 --push # bump the pinned tag # scripts/ci/build_builder_image.sh --no-cache # force a clean rebuild # # The tag is the contract with CI. .gitea/workflows/unit-tests.yml names an @@ -34,7 +34,11 @@ DOCKERFILE="Dockerfile.builder-cpu" # .gitea/workflows/unit-tests.yml; the workflow asserts at run time that the # image it landed in reports this same version, so a drift shows up as a failed # job rather than as a build against the wrong toolchain. -TAG="v1" +# +# v2 added libfftw3-dev: the learned scene-boundary detector's find_library is +# REQUIRED at configure time, so v1 cannot configure this repository at all. +# v3 adds unzip, which pull_artifacts.sh needs to unpack the replay fixtures. +TAG="v3" PUSH=0 EXTRA_ARGS=() diff --git a/tests/test_face_tracker.cpp b/tests/test_face_tracker.cpp index 587a7b1..ca8cc32 100644 --- a/tests/test_face_tracker.cpp +++ b/tests/test_face_tracker.cpp @@ -192,6 +192,13 @@ TEST_CASE("a track within the extinction window is still a candidate", Embedding person = at_sim(0, 1, 0.99f); int id_pre = r.track_of(frame(0.0, 10, 10, person)); + + // Identify it while it is still on screen. Since 1477c53 a DORMANT track is + // only associable if it was owned -- an anonymous one has no actor to + // re-attach to, so it is dropped from the pool a line before the horizon is + // ever consulted. Without this the test cannot reach what it is about. + r.reg->observe(id_pre, /*actor=*/0, /*posterior=*/0.99f, person); + r.track_of(frame(1.0, 300, 300, axis(7), /*is_cut=*/true)); // Back inside the window: the same person continues the same track, so the diff --git a/tests/test_track_registry.cpp b/tests/test_track_registry.cpp index 4c99178..20a9deb 100644 --- a/tests/test_track_registry.cpp +++ b/tests/test_track_registry.cpp @@ -415,15 +415,21 @@ TEST_CASE("a track is not reaped until the evidence clock passes it", } /// TRACES: UT-001 | AR-008, AR-013 | SR-002 -TEST_CASE("a track retired from association is still open to evidence", +TEST_CASE("association and reaping retire a track on the same clock", "[registry][AR-008]") { - // The two clocks answer different questions and must not share an answer. - // Association asks "may this detection link to that track?" on the tracker's - // clock; reaping asks "is that track finished?" and cannot answer until the - // votes are in. Deferring both to the evidence clock was the second half of - // this bug: retired tracks lingered in the candidate pool for as long as the - // matcher lagged, so a new face re-associated onto a long-dead track and two - // people merged into one window. + // Renamed and rewritten for the unified clock. This test used to assert the + // opposite arrangement -- association answered on the tracker's clock while + // reaping answered on the evidence watermark -- and it was written when that + // was true. It is not true now: candidates() and reap_locked() take the SAME + // clock and the SAME threshold, so "retired from association" and "reaped" + // are one event, and the interval the old name described does not exist. + // + // Both halves of the AR-013 bug are visible here. Offering on a LOOSER + // horizon than the reap left retired tracks in the candidate pool for as + // long as the matcher lagged, so a new face re-associated onto a long-dead + // track and two people merged into one window. Reaping on a looser horizon + // than the offer killed tracks whose votes were still in flight. One clock + // makes both unrepresentable. TrackRegistry reg(cfg(/*extinction=*/5.0), disc()); Sink sink; sink.attach(reg); reg.expect_evidence(); @@ -432,21 +438,37 @@ TEST_CASE("a track retired from association is still open to evidence", { auto s = reg.begin_frame(0.0); id = s.create(0.0, axis(1)); + } + // Owned before it goes dormant, for the reason above: an unidentified + // dormant track is not a candidate at any horizon (1477c53). + reg.observe(id, 7, 0.99f, axis(1)); + { + auto s = reg.begin_frame(0.0); s.mark_lost(id, 0.0); } { - auto s = reg.begin_frame(3.0); // inside the window + auto s = reg.begin_frame(3.0); // watermark inside the window CHECK(s.candidates().size() == 1); // still associable } + // The tracker races far past the horizon while the matcher lags. This + // retires nothing, and that is the point of the watermark: the clock that + // decides has not moved, so the track is still offered. { - auto s = reg.begin_frame(50.0); // far outside it - CHECK(s.candidates().empty()); // retired from association... + auto s = reg.begin_frame(50.0); + CHECK(s.candidates().size() == 1); } - // ...but not gone, and still able to receive the votes in flight for it. + // Which is what keeps it able to receive the votes still in flight for it. reg.observe(id, 7, 0.99f, axis(1)); CHECK(reg.dropped_votes() == 0); + + // Only the watermark passing last_seen + extinction retires it -- and the + // same call reaps and emits it, so there is no interval in between. reg.advance_evidence(50.0); + { + auto s = reg.begin_frame(50.0); + CHECK(s.candidates().empty()); + } REQUIRE(sink.claims.size() == 1); CHECK(sink.claims[0].actor_idx == 7); }