Skip to content

fix(ci): skip the fused encoder where triton is absent - #13

Merged
jdsika merged 1 commit into
integration/3.0from
fix/cpu-fused-slot-encoder
Nov 11, 2026
Merged

jdsika merged 1 commit into
integration/3.0from
fix/cpu-fused-slot-encoder

Conversation

@jdsika

@jdsika jdsika commented Nov 11, 2026 •

Copy link
Copy Markdown

What

Pin policy.fused_slot_encoder off at the three call sites that build a Drive policy straight from
the shipped config on machines without triton: the action-selection unit test, the released-driver
eval test, and the inference notebook.

Why

pufferlib/config/puffer_drive.yaml sets fused_slot_encoder: true, and DriveBackbone.__init__
raises when that is set without triton:

ImportError: fused_slot_encoder=true requires triton

GitHub runners have no triton, so Unit Tests and Notebook Execution have been red on every
pull request, including PR #10, which touches nothing but fork-only documentation and tooling. Two
red required checks on every PR means Gate 1 can never be satisfied by anybody, so nothing can be
stacked until this is fixed.

The smoke suite already pins the flag off for exactly this reason
(tests/smoke_tests/test_drive_train.py: "the CPU-only image has no triton"), and so does the
hand-built policy in tests/eval/test_eval.py. The three call sites fixed here were simply missed
— note that test_eval.py pins the flag in the test that builds policy kwargs by hand, but not in
test_policy_builds_from_the_released_driver_env, which deliberately loads the shipped config.
That asymmetry is why the flag survived to CI.

Not fork-specific: fused_slot_encoder: true arrived in upstream Emerge-Lab#601 and is present on pure
upstream/3.0, so upstream's own CI is red the same way.

How tested

  • pre-commit run --all-files
  • make rebuild (required if .c/.h changed — zero new warnings)
  • make test-unit / make test-eval
  • make test-c (C sim logic)
  • make test-notebooks (notebooks touched)
  • make test-docker-smoke (dynamics, rewards, observations, or training loop touched)
  • make -C tests/drive perf (hot paths touched)
  • make test before marking ready (unit + eval + c + notebooks — does not include
    docker-smoke or perf; those are the two boxes above)
$ pre-commit run --all-files
trim trailing whitespace.................................................Passed
fix end of files.........................................................Passed
ruff format..............................................................Passed
clang-format.............................................................Passed

$ python -m pytest tests/unit_tests/test_action_selection.py -q
..........                                                               [100%]
10 passed in 125.10s (0:02:05)

$ python -m pytest -q tests/eval/test_eval.py::test_policy_builds_from_the_released_driver_env
.                                                                        [100%]
1 passed in 53.83s

The eval call site was found by CI, not locally: the first revision of this PR fixed only the
notebook and the unit test, and the Unit Tests check stayed red on
tests/eval/test_eval.py::test_policy_builds_from_the_released_driver_env with the identical
ImportError (run 43017204299).
That run is the fail-before evidence for the third call site; the local run above is the pass-after.

make test-notebooks could not run in this environment — nbclient is not installed in the
checkout's venv. The failing cell was instead reproduced directly: loading puffer_drive, building
the notebook's env with the notebook's own overrides, and constructing DrivePolicy exactly as
05_inference.py does. Before the change that raises ImportError; after it prints
OK: policy built on cpu. The Notebook Execution check on this PR is the authoritative run.

Ran before the change on a clean tree to confirm the failures are pre-existing and not introduced
here: the same two tests fail identically on integration/3.0 without this commit.

New behaviour is covered by a new test / the fix has a test that fails without it:

No new test. This change is a configuration fix to three existing test entry points; the tests that
prove it are test_drive_mean_action_is_exact_in_physical_units and
test_policy_builds_from_the_released_driver_env, which fail on integration/3.0 and pass here.

Stack hygiene

  • forkctl check-commit passes (--chore on a fork-only branch)
  • Conventional Commits subject, rebased on current integration/3.0
== check-commit
  PASS  exactly one commit on the branch
  PASS  rebased directly onto origin/integration/3.0
  PASS  subject is well formed: 'fix(ci): skip the fused encoder where triton is absent'
  PASS  fork-only markers are well formed
  PASS  fork-only state is intact
  PASS  no registered fork-only file in the diff
  PASS  fork-only branch filters are untouched in shared workflows
  PASS  fork-only blocks in shared files are unchanged
== check-commit: OK

Merge gate

  • Gate 1 — CI green on the final commit (no "green except a flake")
  • Gate 2 — adversarial review posted by someone other than the author; findings fixed or
    explicitly accepted in the thread
  • Gate 3 — upstream-readiness proven: forkctl mirror <slug> --sha HEAD passes, i.e. the
    commit cherry-picks onto pure upstream/3.0 and its diff is upstream-clean

Gate 3 result:

== mirror/cpu-fused-slot-encoder
        feature commit d233bb58c  fix(ci): skip the fused encoder where triton is absent
  PASS  cherry-picks onto pure upstream without conflict
  PASS  no registered fork-only file in the mirror diff
  PASS  no fork-only marker added anywhere in the mirror diff
        mirror diff touches 3 file(s):
          notebooks/05_inference.py
          tests/eval/test_eval.py
          tests/unit_tests/test_action_selection.py
== mirror/cpu-fused-slot-encoder: OK

Upstream disposition

  • Mirror PR to Emerge-Lab/PufferDrive requested now — needs a human to verify the final
    title, body and diff first, and the body must carry the authorship disclosure from
    .github/fork/PROCESS.md → Agent authorship
  • Contributable, but we hold it for now, because: this is an upstream defect and a strong
    mirror candidate — upstream CI is red the same way — but opening an upstream PR needs
    explicit human approval of the title, body and diff (§5). Flagging it rather than deciding.
  • Never upstream — fork-only chore(fork): change, Gate 3 exempt

Risk / notes

Behaviour on a machine with triton and a GPU is unchanged at all three call sites: the notebook
keeps the kernel whenever torch.cuda.is_available(), and neither test ever exercised the encoder
— the unit test checks the discrete-to-continuous action table and the eval test checks that
load_policy survives the driver env being released, and the fused kernel has no part in either.

Nothing in the training path changes. The kernel itself is untouched.

Follow-up worth considering separately, not done here: the mismatch is structural, and this PR
needing a second round to find a third call site is the evidence. Any new test or notebook that
builds a policy from the shipped config will hit this again, because the shipped default is
GPU-only while CI is not. A shared test helper that yields CPU-safe policy kwargs would remove the
whole class, but that is a refactor with its own review surface.

The shipped puffer_drive.yaml sets policy.fused_slot_encoder=true, and
DriveBackbone raises ImportError when that is set without triton. Two CI
jobs build a policy straight from the shipped config, so Unit Tests and
Notebook Execution have been failing on every pull request:

  ImportError: fused_slot_encoder=true requires triton

The smoke and eval suites already pin the flag off for exactly this reason;
these two call sites were simply missed. test_action_selection only exercises
the discrete-to-continuous action table, which the fused kernel has no part
in, so it pins the flag off outright. The inference notebook keeps the kernel
whenever it has a CUDA device and drops it otherwise, which is also what a
reader running the notebook on a laptop needs.

Neither change touches the training path or the kernel itself: with triton
and a GPU present, both call sites behave exactly as before.
@jdsika
jdsika force-pushed the fix/cpu-fused-slot-encoder branch from c43f518 to d233bb5 Compare November 11, 2026 12:14
@jdsika
jdsika merged commit d91b4b2 into integration/3.0 Nov 11, 2026
12 checks passed
@jdsika
jdsika deleted the fix/cpu-fused-slot-encoder branch November 11, 2026 16:30
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