Skip to content

[Fix] Conductor SIGINT, piecewise KV capture, and batched AC rollout split - #176

Merged
NSagan271 merged 4 commits into
mainfrom
main-fixes
Jul 18, 2026
Merged

[Fix] Conductor SIGINT, piecewise KV capture, and batched AC rollout split#176
NSagan271 merged 4 commits into
mainfrom
main-fixes

Conversation

@merceod

@merceod merceod commented Jul 18, 2026

Copy link
Copy Markdown
Collaborator

What does this PR do?

Three unrelated fixes found while validating #164:

  • api_server: _shutdown_conductor_process called send_signal(SIGINT) on a multiprocessing SpawnProcess, that's subprocess.Popen API, so every shutdown raised an AttributeError (caught and logged) and the conductor was always SIGTERM'd without running its graceful path. Use os.kill instead. SIGINT-ing the api server now yields "Shutting down conductor..." and a clean tree exit.
  • engine: the piecewise runner still instantiated BatchedCacheManager directly; it has been an ABC since [WIP] cosmos3: add generator model scaffold (+reasoner implementation) #121, so every uses_kv_cache=True piecewise capture failed at startup (warn-only, easy to miss) and vjepa2_ac's block_loop graphs were never captured. Route through create_cache_manager() like the other call sites. All four buckets (bs=1/2/4/8) capture again.
  • vjepa2: VJepa2ACRolloutPredictorSubmodule.forward_batched sliced the output list instead of the tensor when splitting per request, so with B≥2 the first request got the whole batched tensor and the rest got an empty list. Cconcurrent AC rollouts then crashed (Per-request tensor has leading dim 2 != 1) or truncated. Slice rows like the sibling submodules do. New CPU-only regression test test_ac_rollout_batched.py fails on the old code.

How was it tested?

Verified end-to-end on vjepa2_ac: sequential outputs bit-identical to before the change (sha-matched), and 6 concurrent rollouts at concurrency 3 all complete where they previously crashed.

Checklist

  • ruff check . passes
  • Added or updated tests / docs where relevant

@NSagan271 NSagan271 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, also fixed a few other vjepa issues

@NSagan271
NSagan271 merged commit b2a8eda into main Jul 18, 2026
2 checks passed
@NSagan271
NSagan271 deleted the main-fixes branch July 18, 2026 18:52
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.

2 participants