0.8: the rewritten server - #272
Open
JadenFiotto-Kaufman wants to merge 61 commits into
Open
Conversation
Brings the 0.8 rewrite onto the ndif repo, replacing the previous server. The controller, its cluster/node/evaluator model and the detached-Ray-actor deployment mechanism carry over and evolve; what changes substantially: - How untrusted code runs. The previous server executed a user's traced block in-process in the model actor, contained by a six-layer in-process Protector (import interception, a module/builtin whitelist, attribute guards, an audit hook, restricted compile/exec). That whole approach is gone. Untrusted blocks now run in a separate runner process, one fresh process per request, with nnsight's interleaver split across a Unix socket so the model never leaves the actor. Trusted blocks still run in-process; request.trusted, stamped at ingress from the API key, is the fork. - Client transport. Socket.IO with a Redis-backed cross-process manager is replaced by a plain FastAPI websocket over Redis pub/sub. - Telemetry. OpenTelemetry tracing (collector, Tempo, common/tracing) is removed. Logs go to Loki, NDIF's own metrics to InfluxDB, Ray's to Prometheus, with nine provisioned Grafana dashboards. - Packaging and dev loop. One image selected at runtime by NDIF_SERVICE with the `ndif` CLI as entrypoint; justfile and a pinned requirements.txt replace the Makefile and uv.lock. nnsight is an ordinary dependency again rather than vendored, bind-mounted from a local checkout for client-side work. Adds CLAUDE.md as an agent router over a 61-page docs/ tree: concepts, operating, runbooks, developing, reference, errors and gotchas. Dropped here and to be re-added deliberately: .github workflows, .env.example (it described the previous variable scheme), NDIF.md (superseded by docs/), telemetry/, scripts/ and uv.lock. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The runner now takes the model key as a second argument and builds an undispatched meta model from it at startup, using its _remoteable_persistent_objects() as the map IPCCloudUnpickler resolves against. Module:<path>, Tokenizer and Pipeline ids therefore deserialize to real objects in the runner instead of None; "Interleaver" is still checked first so the IPC interleaver isn't shadowed. Pool carries the runner args through to spawn so every runner it starts gets the key. quiet=False for now, so runner stdout/stderr reaches the actor's worker log while this lands.
The pool that pre-warms sandbox runners was fixed at 2. `pool_size` existed as a kwarg but nothing ever passed it, so it was effectively hardcoded. Two costs set the right value, both measured on a g4dn.xlarge: a cold runner spawn (python + torch + nnsight import) takes ~4s, while executing an already-warm request takes ~0.7s. Refills run concurrently, one thread each, so the pool keeps up only if it is at least spawn/execute ~= 6. At 2 it could not. A saturated queue drained the pool immediately and settled into an alternating pattern -- one request served warm, the next paying the full ~4s spawn inline -- averaging 2.1s of service time against 0.29s on the trusted (in-process) path. Untrusted throughput was 0.46 rps where trusted was 3.5 rps, on a GPU that was idle in both cases. Raising it is not free: each warm runner holds ~420 MB (PSS), so 7 is ~2.9 GB per model actor, and concurrent refills contend for CPU on the actor's node. Hence NDIF_SANDBOX_POOL_SIZE, so a memory- or core-tight node can turn it down. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
reconcile() only ever shed replicas -- it computed set(self.replicas) - current and cancelled those, never the reverse. Nothing else adopts either: ensure_started no-ops on a non-empty pool, and start() only runs on an empty one. So a replica added while a model was already serving was never given work. Measured: deploying a second gpt2-medium replica changed throughput not at all (3.21 rps). Restarting the API -- which rebuilds the Processor and makes start() adopt both -- took the same two replicas to 5.99 rps, an 87% gain with nothing changed on the GPU. An operator scaling a hot model got zero benefit and no warning. Two details worth preserving. adopt() runs as a task rather than inline because reconcile is awaited by the dispatcher's events worker and Replica.wait has no timeout -- waiting inline would wedge that worker, and every later reconcile and `ndif kill`, on a single unready actor. And adoption is skipped while status is PROVISIONING/DEPLOYING, because a start() already in flight adopts the same controller list and the two would race into two workers on one replica. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
enqueue reported self.queue.qsize() as the new request's position unconditionally. For an appended request the depth is the position, so this looked right -- which is why it went unnoticed. For a prepended one it is exactly wrong: the request is at the front and was told it was last. Both prepend callers were affected. A priority-tagged request that jumped five queued requests and ran second was told "Added to Queue at position 6", and an evicted replica handing its in-flight request back reported position 3 before resuming first. Users with a priority key saw their key apparently doing nothing. reply()'s "Moved to position N" message was always correct -- it enumerates the real deque -- which is why a re-queued request emitted a contradictory pair. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Node.evict has two outcomes, and they disagreed about what happens to a request that was running at the time. If the node had no CPU cache room the actor was removed, the queue's Ray call raised ActorDiedError, EVICTED_ERRORS caught it and the request was re-queued and re-run -- entirely transparent. If there *was* room the replica was demoted to WARM instead, and to_cache() cancels the in-flight execution as its first action, before the weights move. That path responded ERROR to the user directly, so run() never raised, EVICTED_ERRORS never fired, and the working re-queue path was bypassed. The polarity was backwards: the tidier eviction -- the one that keeps the model in CPU RAM for a fast restore -- destroyed the request, while the wasteful one was silently retried. And demote is the common case, so anything that makes the WARM cache more effective made the failure more frequent. Reproduced by filling the GPU and deploying a pinned replica that forced an eviction: all three in-flight and queued requests died. The demote path now raises CachedActorError, so both outcomes land in EVICTED_ERRORS and both re-queue. Crucially the reason is recorded rather than inferred. Treating "the kill switch fired" as meaning "eviction" would hold only while to_cache is its sole caller; any other cancel would then be re-queued, and since re-queues go to the *front* of the line, a deliberately cancelled request would re-run forever. cancel() takes a reason, to_cache passes KILL_REASON_PREEMPTED, and anything else -- including the default of none -- stays terminal. Retry is opt-in. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Ray retains a DEAD record for every actor it has ever run, and status() maps
DEAD to UNHEALTHY. The enrichment loops below only fill in names the controller
still tracks, so a dead actor it has forgotten stayed a bare
{"application_state": "UNHEALTHY"} with no repo_id -- and nothing ever removed
it.
They accumulate monotonically, one per evict or restart, for the life of the Ray
cluster. Watched climb 1 -> 5 -> 7 -> 9 over a day of testing, at which point
they outnumbered the real entries 9 to 4. Deploys add none; only teardowns do.
Any consumer reading `deployments` and assuming a repo_id sees mostly junk, and
"how many models are deployed" answers 13 when the truth is 4.
Only orphans are dropped. A dead actor the controller *does* still track was
enriched, and that combination is a real signal worth surfacing: the controller
believes a replica is deployed while its actor is gone. The filter therefore
keys on a missing model_key, not on the state alone, and runs before the COLD
synthesis so downloaded-but-not-resident models are unaffected.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Verified against the test stack: a replica-targeted evict that demoted 8c645 to WARM still killed the request running on it, with `Replica evicted.` -- even though the actor-side fix (to_cache raising CachedActorError) was in place and the actor stayed ALIVE. A demote destroyed the in-flight request by two independent routes. The actor one is fixed. The other is here: the out-of-band evict emits reconcile_model, the replica is no longer in get_deployment (which returns HOT only, so a WARM replica reads as gone), and reconcile shed it via Replica.cancel, which errors the in-flight request terminally. That route won the race. Rather than teach cancel to tell a retryable shed from a deliberate kill, don't shed at all. The worker already handles a vanished replica correctly by itself: the next dispatch raises one of EVICTED_ERRORS (CachedActorError when demoted, ValueError/ActorDiedError when removed), dispatch hands the request back to the front of the queue and clears self.task, the loop condition flips, and the finally drops the replica and re-provisions. That is the documented eviction behaviour, and it loses nothing. The cost is a replica whose worker is idle lingering in the pool until traffic touches it -- harmless while the queue is empty, and self-correcting on the next request, which pays one wasted dispatch and is then re-queued and served. Replica.cancel keeps its remaining callers, `ndif kill` and purge, where erroring the request is the correct semantic. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
nnsight is installed from a git URL (requirements.txt tracks the 0.8 branch, which has no PyPI release), and pip needs `git` present to resolve it. Without this the image build fails at the install step. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Three different user mistakes -- a mistyped repo_id, a gated repo with no
HF_TOKEN, and a model too big for the cluster -- all produced the same string:
"Error provisioning model. Please try again later." Each needs a different fix
and none of them is waiting, so the one piece of advice given was the only one
guaranteed not to work.
The good message already existed. The controller returns it, Replica.provision
raises it, and start() logs it with exc_info before replacing it with the canned
line. Recovered from the API log during testing:
Repository Not Found for url: https://huggingface.co/api/models/...
Please make sure you specified the correct `repo_id` and `repo_type`.
You are trying to access a gated repo.
Make sure to have access to it at https://huggingface.co/meta-llama/...
HuggingFace phrases those for end users already, naming the repo and the page to
visit. The operator path has always surfaced its equivalent
("CANT_ACCOMMODATE: placed 0 of 1 new replicas ..."); only the user path
discarded it.
provision now raises DeploymentError rather than a bare Exception, and start()
forwards that -- and only that -- as "Could not deploy this model. <reason>".
Any other exception is an internal fault whose text would leak implementation
detail and tell the caller nothing, so it keeps the generic message. The reason
is capped so a pathological error string can't become the websocket payload.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Deserialization and execution shared one `except` in the sandbox runner, so a corrupt payload came back looking exactly like a user-code failure: a traceback through NDIF's own frames, naming this file's path and IPCCloudUnpickler, ending in `ZstdError: error determining content size from frame header`. The caller could act on none of it, and it also broke the triage rule the runbook documents -- "a traceback means their code, a fixed sentence means ours" -- because the failure happens before their code exists. The two phases are now separate. A deserialize failure produces a plain sentence naming the underlying exception class; execution keeps the existing traceback formatting untouched. Both paths say the same thing. The trusted path deserializes in the model actor and the untrusted one in the runner -- different processes, and only text crosses the runner's socket -- so the message lives in common/errors.py, which both import. The actor raises PayloadError, which format_error renders as that sentence and marks non-fatal (the request is malformed; the actor is fine); the runner, which cannot send an exception object, sends the same text. Care in the runner: `run` does not return its terminal event, it falls through to `conn.send(*terminal)`. An early return there would skip the send and hang the request forever, so the execution block is nested in the deserialize try's `else` rather than short-circuiting. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Adds the architecture-mismatch case and folds it in with the payload case, so
every way reading a request can fail is decided by one method that both
server-side deserialization sites call.
The mismatch: a block records each module it touches as a persistent id,
Module:<path>, resolved against the server's live model, so a path the server's
tree lacks means the two trees were built differently -- nearly always a
transformers version difference, since that decides a checkpoint's module
layout. Reproduced by tracing gpt2-medium's transformer.h[20] and submitting it
against gpt2: the in-process path raised
UnknownPersistentIdError: Module:model.transformer.h.12
which names a module the caller never mentioned (the block references the whole
envoy tree, so it fails at the first path where the trees diverge, not the layer
they were reaching for). That boundary is the useful part, so the new message
quotes it, says version drift is the likely cause, and hands over
`from nnsight import ndif; print(ndif.compare())` -- whose CRITICAL_PACKAGES is
exactly {nnsight, transformers, torch}.
Three things made this land cleanly:
- The exceptions build their own messages from the facts they are given, so
there is one place per failure deciding both what it is and how it reads. The
free message-builder functions are gone. ArchitectureMismatchError is a
*sibling* of PayloadError under a shared RequestError, not a subclass -- in a
mismatch the payload is perfectly readable, the environment differs -- and
format_error keys off the base.
- Classification moved into BackendRequestModel.deserialize, which removed the
error handling from the actor's execute() entirely, including the re-raise
that existed only to stop the generic wrapper clobbering a specific message.
That problem cannot occur with a single classifier.
- The runner can now call that same method. It previously could not: importing
ndif.common.schema.request connected Redis and pulled in the InfluxDB client,
which would have happened in every pooled runner process. Those three imports
are used only by _advance_status/respond/arespond -- which the runner never
calls -- so moving them into those methods makes the module side-effect free
(verified: no threads, no redis, no influxdb_client on import). This also
stops request.py dragging Redis into any process that merely imports it.
Nothing is done for the sandbox path's own detection: its unpickler resolves
every module id to None rather than raising, so a mismatch there still surfaces
late as BrokenPipeError. Left alone deliberately -- once the runner is given
real persistent objects it will raise UnknownPersistentIdError itself and pick
up this message through the shared classifier, with no second detection site to
disagree with the first.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
EVICTED_ERRORS never matched a CachedActorError coming back from an actor, so a
HOT->WARM demotion has always errored the in-flight request instead of
re-queueing it. This is the root cause of that failure -- deeper than the
to_cache cancel or reconcile's shed, both of which were fixed first and were not
enough on their own.
An exception raised inside an actor arrives wrapped in a
ray.exceptions.RayTaskError. The dual RayTaskError-plus-cause class that would
satisfy isinstance is only built when as_instanceof_cause() is applied, and over
Ray Client -- which is how the dispatcher connects -- it is not. Observed on the
test stack: the actor log showed
base.py, line 276, in run
raise CachedActorError(... is cached (WARM).)
while the dispatcher logged error_type=RayTaskError and fell through to the
generic handler. Had the dual class been built the type would have read
RayTaskError(CachedActorError).
dispatch now classifies through is_evicted_error, which checks the exception and
then its .cause. The two branches merged into one handler since the evicted case
returns. Verified end to end: evicting a replica mid-execution now re-queues the
running request at position 1, re-provisions, and completes it, with no error
reaching the caller.
The docstring on CachedActorError asserting the isinstance behaviour was wrong;
the queue-internals doc is corrected too.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
It claimed the RayTaskError wrapper "still satisfies isinstance(e, CachedActorError)". It does not over Ray Client, and the queue believed it: `except EVICTED_ERRORS` never matched, so every HOT->WARM demotion errored the in-flight request instead of re-queueing it. 42c6cc5 fixed the matching and said it corrected this docstring; it corrected docs/developing/queue-internals.md and missed this one, which is the copy that caused the bug. Now says not to catch by type, and points at queue.replica.is_evicted_error. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
contributing.md asks for present tense ("no 'this used to'"), and the pages
touched by the recent queue/eviction/error work had drifted into narrating the
change rather than describing the system. Rewritten to say how it behaves, with
the reasoning kept where it stops someone reintroducing a problem — why the
demote path is easy to break, why shedding in reconcile would lose work, why a
bare isinstance across the Ray boundary matches nothing.
Two entries added to the CLAUDE.md cheat-sheet: exceptions from an actor cannot
be caught by type over Ray Client, and the sandbox pool's per-actor memory cost.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
An exception raised inside a Ray actor reaches the caller as a RayTaskError, whose own type says nothing about what failed. Every log line at the Ray boundary therefore read error_type=RayTaskError -- true, and precisely useless. That is what hid the eviction bug: the actor raised CachedActorError exactly as designed and the dispatcher's own logs agreed nothing unusual had happened. error_type_name reads .cause and renders RayTaskError[CachedActorError], applied at the four sites that log an exception crossing that boundary (replica dispatch, both processor handlers, the dispatcher's error drain). A plain exception, or one whose cause matches its own type, logs the bare name as before. The failure this makes visible is the general one, not just eviction: any exception the queue wants to recognise by type is invisible in the logs while it is being mishandled. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The field list is what Grafana queries key off, so how error_type is built belongs there: RayTaskError[CachedActorError] rather than a bare wrapper name. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Three conflicts, all where 0.8 rewrote code this branch also touched: - nns.py run(): took 0.8's split error handling (RequestError from BackendRequestModel.deserialize reported without a traceback, user-code failures still formatted here) and kept this branch's persistent_objects argument. Deserialization now goes through BackendRequestModel, so the unpickler docstring says so, and notes that the runner's None fallthrough bypasses 0.8's ArchitectureMismatchError classification. - model.py: kept 0.8's NDIF_SANDBOX_POOL_SIZE default alongside this branch's runner_args=[model_key]. - sandbox-internals.md: kept 0.8's pool-sizing rationale and folded the runner_args paragraph and the silent-spawn-failure gotcha in after it. Also refreshed adding-a-model-actor.md, whose pool passage still said the size is always 2 and nothing can set it.
An unpinned branch means every image build silently ships a different client library. Observed: two builds four hours apart moved nnsight three commits, through intervention/serialization.py, tracing/util.py and tracing/tracer.py -- the serialization path the server depends on. Nothing in the build output says so, and the two images are indistinguishable by tag. That matters beyond reproducibility. The server runs traced blocks its clients serialized, and a block records each module it touches as a persistent id resolved against the server's model tree; drift across that boundary surfaces to users as an architecture mismatch, not as a version error. Pinned to ad939e9bb, which the deployed test image was built from and verified against. Bump it deliberately, and re-run the deserialization checks when you do. The header records how to recover what any given image actually resolved (`pip freeze` inside it) and what this pin produces. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
One image serves every NDIF service; the container picks its role from NDIF_SERVICE at runtime. Tagging deliberately matches ndif-aws/push.sh, the manual path onto the same repositories: every build produces an immutable sha-<7> tag alongside the mutable latest. The ECR lifecycle policy keys on `sha-*` and expires untagged images after 14 days, so an image pushed without its sha tag becomes unreferenceable garbage two weeks later. Authentication is OIDC (role-to-assume) rather than static keys, and the job takes only `contents: read`. Redeployment is manual and opt-in. The previous workflow force-redeployed NDIF-Prod/API on every push to main, which is how prod's API and Dashboard drifted onto different builds -- both track :latest, but only one was ever restarted -- and it deployed prod unattended, which the ndif-aws ground rules forbid. The deploy job now redeploys every service running the image, and only when someone asks. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Any optimizer loop inside a remote session silently did nothing. Gradients were real and the parameters visibly moved, but the model never saw the new values, so the loss did not respond -- the LoRA tutorial trains for 500 batches and ends where it started. execute() wraps the whole request in one torch.autocast region. Autocast's cache keeps the half-precision copy it made of each fp32 leaf that requires grad and reuses it for the rest of the region. That is correct for a single forward pass, and wrong for a region spanning many: every forward after the first runs against the snapshot taken before the first optimizer.step(). The tell is a matmul that returns exactly zero -- WB.norm() reads back as 166 while (A @ WB).norm() is 0.000, because the cached bf16 copy of WB is still the zeros it was initialised with. Recomputing the same product from WB.detach().float() gives the real answer, since a fresh non-leaf misses the cache. cache_enabled=False costs nothing here: the served weights are already loaded in self.dtype, so there is no weight cast worth reusing. Verified on gpt2 (loss responds from step 1 where it was previously frozen) and by re-running the LoRA tutorial against Llama-3.1-70B, which now falls from 35.45 to 0.71 over 500 batches. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The default PyPI wheel resolved to 2.13.0+cu130, a CUDA *13* build. Any host whose driver predates CUDA 13 gets torch.cuda.is_available() == False and "the NVIDIA driver on your system is too old", and the model actor then reports cuda_memory_bytes: 0 rather than failing outright. Seen on a driver 555.42.02 (CUDA 12.5) box; a 570.86.15 (CUDA 12.8) box fails the same way. Same torch version the image already resolved, built against 12.6 instead. Any 12.x build runs on any 12.x driver >= 525 (CUDA minor version compatibility), so this widens the set of hosts the image works on rather than narrowing it. The +cu126 local version tag is what forces the pytorch index over PyPI's cu130 wheel. This trades away some of the deliberate looseness the file documents: torch is left unpinned so nnsight can resolve it. Drop this commit if the deploy target is on a CUDA 13 driver. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Blocks that evaluate over a benchmark should load it on the server rather than pickling it into the request: a HuggingFace Dataset is memory-mapped, so shipping one puts an arrow file path in the payload that only exists on the client, and the worker fails with FileNotFoundError. `from datasets import load_dataset` inside a session is the pattern nnsight documents for that (docs/patterns/remote-dataset-sweep.md), which only works if the server provides the package. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`ndif deploy` exited 0 on failure. A model that can't be placed is recorded in deploy_lib's return value as status ERROR/PARTIAL and printed as a `✗` line — it is never raised, because deploy_lib is shared with the dashboard's HTTP endpoint and reports outcomes as data. The command only aborted on exceptions, so it fell off the end and click exited 0. A failed deploy in a script looked like a success. Check the statuses the library already returns and abort if any model isn't READY, the way evict already does. Verified both ways against a live stack: a deploy with no room prints `✗ 1 of 1 model(s) not ready.` and exits 1; a successful one still exits 0. Also from the same agent run, two runbooks that don't work as written: - model-oom-on-deploy.md told you to grep `just logs ray`. The controller is a Ray *actor*, so its output never reaches container stdout — it is in /tmp/ray/session_latest/logs/worker-*.out inside the ray container. That grep returns nothing, twice, while the lines that explain the failure sit there. Nothing else in the docs says where actor logs live. - deploy-and-pin-a-model.md is the page CLAUDE.md calls "the procedure" and gave bare `ndif deploy` with no indication of where it runs from. Says now that it reaches the controller from anywhere NDIF_RAY_ADDRESS is reachable, with the compose form as one example. CANT_ACCOMMODATE's row also notes that the message carries no arithmetic and that an NDIF_DEFAULT_PADDING_* override is invisible to every page quoting the defaults — an agent spent most of a session on exactly that. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Two things that were wrong to put in requirements.txt. **torch.** dcd52eb pinned `2.13.0+cu126` there, which is a property of the *host's* CUDA driver, not of this repo — the pin was right for the box I was on and wrong to commit. It moves to docker/Dockerfile behind build args, defaulting to the latest torch on the cu126 index: ARG TORCH_SPEC=torch ARG TORCH_CUDA=cu126 docker build --build-arg TORCH_CUDA=cu128 . # another CUDA line docker build --build-arg TORCH_SPEC="torch==2.11.0" . # pin the version cu126 rather than PyPI's default because any 12.x wheel runs on any 12.x driver >= 525 (CUDA minor version compatibility), while the default is now a CUDA *13* build that refuses to initialise on a 12.x driver — and does so quietly: torch.cuda.is_available() returns False and the actor reports cuda_memory_bytes: 0 rather than failing. --index-url rather than --extra-index-url is what keeps pip off that wheel; this layer installs only torch, so narrowing the index is safe. Installed first and in its own layer, since it is the largest single download. This is also how the upstream images do it — pytorch/pytorch publishes one tag per (torch, cuda, cudnn) combination from a single Dockerfile, NGC likewise — so a published NDIF image should be one tag per CUDA line rather than one image trying to cover every driver. The build args make that a CI matrix. **`ext`.** f02dfc0 added `datasets` to requirements.txt, which put a package nothing in ndif imports next to ones it does. The distinction worth encoding is that these are the packages *user-submitted blocks* may import: the server has to provide them or a block dies with ModuleNotFoundError on the far side. Taken from the sandbox import whitelist on dev (services/ray/nn/security/whitelist.yaml), minus everything already pulled in by transformers/datasets/torch. That leaves datasets, einops, pillow, diffusers, scipy, nnterp. All 25 third-party modules the whitelist names are now importable in the image, up from 20. The Dockerfile reads the list out of pyproject.toml rather than running `pip install ".[ext]"`, which is a silent no-op — pip sees ndif already installed at this version and never resolves the extra. Reading it keeps pyproject the one source of truth, so `pip install ndif[ext]` outside Docker gets the same surface. The pattern's failure mode is drift: the whitelist says what a block is *allowed* to import, `ext` says what is *installed*, and a name in one but not the other is a bug either way. Worth a test asserting every whitelisted top-level module imports, once the whitelist lands on this branch. Verified: the default build resolves torch 2.13.0+cu126 with cuda available, 25/25 whitelist modules importable, gpt2 deploys, and the nnsight skills suite passes 135 tests against the rebuilt image. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Three views the deployed prod Grafana has that this dashboard set was missing.
Every query here was run against the live prod datasources (and the local
compose stack) before being committed, not just written.
**Cluster VRAM headroom** (ndif-ray). The Ray dashboard plotted
`ray_node_gram_used` in raw MB with no denominator, so a full 40GB card and a
quarter-full 80GB card looked identical and "can we fit another model?" was
unanswerable. Adds a row with the capacity side: GPU count, total/used/free
VRAM, a cluster utilization gauge, per-GPU utilization *as a percentage* so
mixed 40/80GB nodes compare, and instant bargauges for current VRAM and compute
per GPU — VRAM full with compute at zero is an idle resident model, i.e. an
eviction candidate. Existing panels shift down 13 rows; no gridPos overlaps.
**API keys -> user** (ndif-users). `api_key` is a tag on all five measurements
and a field on every request log, but the dashboards only ever pivoted on
`email`, and "Selected user" showed a *count* of keys, never the ids. So a key
found in a log could not be traced to a person. Adds prod's
`SELECT users.email, keys.key_id ...` panel, plus tags and a search box.
**Exception triage** (ndif-errors). "Errors by type" groups on `error_type`,
which the server only ever sets to cancelled/preempted/timeout — four
system-level values, none of them user code. Meanwhile logging_setup emits
`exception_type` / `exception_message` / `stacktrace` on every logged exception
and nothing queried them. Adds a row that does: rate by type, top types, a
distinct-messages table, and a stacktrace stream keyed off a new `exception`
variable.
The messages table is the point. It reduces a traceback to its final line, then
masks uuids, hex ids, byte sizes and digits, so one bug hitting 400 request ids
collapses to one row with a count of 400. Two details found by running it
against 7 days of prod logs:
- the last-line reduction also peels the sandbox's RunnerError wrapper off the
user's real exception, turning an opaque `RunnerError: 9` into separate
NameError / AttributeError / IndexError / TypeError / ZeroDivisionError rows;
- `exception_message` ends with a newline, so the greedy `(?s).*\n` ate the
whole string and produced empty rows until a trim stage was added first.
The masking regexes sit inside a Go template string literal, where `\b` means
backspace and `\.` is an invalid escape — hence `[.]` and no word boundaries.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A >1024-token prompt on gpt2 raises a device-side assert, which poisons the CUDA context; the actor detects that (`_is_unrecoverable_cuda_error`) and kills itself so Ray respawns it with a fresh context (`max_restarts=-1`). That part works. What didn't: for the ~6s the reload takes, Ray answers every call with `ActorUnavailableError`, which is not in `EVICTED_ERRORS`, so each queued request fell through to the generic handler and was errored to the user. One overflow cost 16 user-visible failures, all of them recoverable — the same replica id was seconds from being back. `ActorUnavailableError` is deliberately *not* added to `EVICTED_ERRORS`: that path re-queues and drops the replica, and neither is right when the same replica is coming back. Ray also warns the task "may or may not have been executed", so a blind re-queue risks double execution. Instead the worker parks on `wait()` until the replica is serving again. Nothing is re-queued, nothing is dropped, and queued requests simply wait. `wait()` grows the one case it was missing. `ActorUnavailableError` means there exactly what `ValueError` already meant — "not yet" — so it belongs in that tuple rather than in a second waiting loop. The subtlety: over Ray Client, which is how the dispatcher connects, `__ray_ready__` raises immediately instead of blocking through the restart the way it does for a driver-mode caller, so it has to be polled. A driver-mode probe suggests otherwise and will mislead you. No timeout: `max_restarts=-1` means Ray keeps bringing the replica back, so any bound would be a guess that eventually fails a large model mid-reload. A deliberate cancel still unwinds the worker, since `CancelledError` is a `BaseException` and propagates through `wait()`. `is_evicted_error` / `is_restarting_error` are inlined into `dispatch`, which is the only caller of either. The `.cause` unwrap stays with the eviction check and keeps its comment — it is load-bearing, and a bare isinstance there silently matches nothing. The restart check needs no unwrap: Ray raises that one itself, so it arrives bare rather than inside a RayTaskError. Measured with 12 requests fired into the restart window: 11/12 succeed, up from 0/12. The one loss is the request already dispatched when the actor went down, which can't be avoided without a health-check round-trip per request. Docs that named the removed helpers are updated, along with three stale file:line citations in the caught-exceptions table. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Found by killing a model actor with ``ray.kill(no_restart=True)`` under load — an 8B replica died permanently and the model became unusable until an operator intervened, with the controller reporting 29.0/47.4 GB free against 620 MiB actually in use. No fix here, just the note: the chain runs from a dead replica record the controller never drops, through ``get_deployment`` handing that id to the queue, to ``Replica.wait`` neither returning nor raising on it — which blocks the healthy replicas behind it in ``Processor.start``'s list and makes the purge that would error the queued users unreachable. The comment records the two traps for whoever picks it up (key off Ray's actor state, not a failed name lookup; drop rather than re-provision) and that a reaper prevents the wedge but cannot cure one, since ``Replica.wait`` is pinned to a replica id. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`priority` was implemented as `deque.appendleft`, which made the priority group
LIFO: a closed-loop client that wins the head keeps winning it. Measured with 16
clients, five of them priority: two clients completed 73 requests each while the
other fourteen — including three *other* priority clients — completed one apiece
in seven minutes. p95 was 417s.
The same prepend blinded the autoscaler, which read the head's wait to decide
whether to scale. The head is the newest prepended request, so its wait is ~0
exactly when the queue is worst. Over that same run it fired once, reporting
`wait_s=399.85` — it saw a 400s-starved request by luck, not by design.
Ordering is now a key rather than an insertion side, in a new `RequestQueue`
wrapping an `asyncio.PriorityQueue`:
(rank, enqueued_at, seq)
rank 0 priority, re-queued rank 2 normal, re-queued
rank 1 priority rank 3 normal
The group is doubled so `prepend` is a sub-rank inside it; a plain "+1" would
collide a re-queued normal request with a fresh priority one. Every tier
tiebreaks on `enqueued_at`, so each is FIFO and nothing starves *within* a group
— including the re-queue tiers, whose occupants are bounded by the in-flight
count. `seq` stops tuple comparison before it reaches the `BackendRequestModel`,
which is not orderable.
Re-queues would sort to the front of their group anyway, since they keep their
original `enqueued_at`. The explicit rank is kept so that is stated rather than
emergent from timestamp preservation.
`prepend` is now only for re-queues; `Dispatcher.dispatch` no longer passes
`prepend=request.priority`.
The class wraps the queue rather than exposing it because five call sites read
`_queue` directly, and a heap's list is only partially sorted — reading it raw
returns a plausible-looking wrong order. `snapshot` (service order, for position
replies and status), `oldest`, `position` and `remove` own the heap invariants.
Queue-position replies are now read back from the queue instead of assuming
depth, which was only ever right for a plain append.
Autoscaling reads `oldest()` — the longest wait across *both* groups — rather
than the head. It is a scan, not a maintained watermark: O(n) on a queue of tens
once per tick, against an invariant every enqueue, dequeue and re-queue would
have to honour.
Results on the same 16-client workload: the five priority clients go from
{1, 1, 73, 73, 1} to {67, 71, 66, 67, 68}, priority throughput rises from 149 to
339 completions, and the autoscaler now trips at `wait_s=33.84` and again at
`159.84`. With no priority traffic all 16 clients complete exactly 15 each at
p95 23.6s, unchanged.
What this does *not* fix: strict priority still starves normal traffic under
saturated priority load — that is the semantics, not a bug, and there is no
aging. Autoscaling is the only relief and it caps at three replicas.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A runner could outlive the actor that spawned it. The graceful paths already stop it — `cleanup` -> `discard_sandbox` after every request, `interrupt` on a timeout or cancel — but eviction is `ray.kill(no_restart=True)` and an unrecoverable-CUDA restart is `ray.kill(no_restart=False)`. Both SIGKILL the actor, so no Python cleanup of any kind runs. A runner executing a runaway block at that moment is orphaned and keeps spinning at 100% of a core until the container restarts, invisible to `ndif status` because its deployment is gone. Observed: two orphans holding ~210% CPU with zero deployments. One request produced both, because an evicted request is re-queued by design and starts a second runner while the first is never stopped — so a runaway block leaks one process per eviction. Polling `getppid` rather than `prctl(PR_SET_PDEATHSIG)`. The signal was the obvious choice and it is wrong here: it fires when the parent *thread* exits, and `Pool.refill` warms every runner on a short-lived `threading.Thread` while `acquire` spawns from the per-request execution thread. Armed with prctl, every runner is killed seconds after it is created — a mixed-load run failed all four untrusted users with connection-refused while both trusted users passed, since only the sandbox path goes through the pool. Polling is thread-agnostic, and a pure-Python runaway still yields the GIL on the interpreter's switch interval so the watchdog keeps getting scheduled while user code spins. Armed in the runner after exec rather than in a `Popen(preexec_fn=...)`, since a fork-time callback in the multi-threaded actor is a documented deadlock risk. Verified by tracking the pid through an eviction: a spinner at 129.9s CPU with ppid 3289 is gone within 20s of its actor being evicted, where before it survived indefinitely. Note the request itself is still re-queued and resumes on a fresh replica — with no default timeout it will spin there too, but attached to a live actor rather than orphaned. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Both execution paths captured stdout only, so anything a block wrote to stderr went to the model actor's log and the person who caused it never learned of it. `warnings.warn` writes there -- which meant a library cautioning a user about their own trace was visible to the server operator and nobody else. nnsight's remote protocol has no WARN status; LOG is the only channel to a client, and it is fed by this redirect. So forwarding stderr is what makes a server-side warning reachable at all. Verified end to end: a `.source` read on a tensor-parallel replica now returns its caveat to the client as a LOG line, where before the same run printed a half-width tensor in silence. Two LogStreams rather than one shared: each buffers a partial line until its newline, so sharing would interleave half-written output from both. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The base actor spreads a model over its GPUs with accelerate -- whole layers per card, run one after another. This one shards *within* each layer using transformers tensor parallelism, so the GPUs work on the same layer at once and a model too big for one card can be served. That needs a process per GPU, and the actor **is rank 0**: it holds a shard, spawns the other ranks, and runs the user's block itself. Being a rank rather than a coordinator is what keeps this small -- run()'s statuses, timeout race, metrics and upload are inherited untouched. Every rank runs the block. Whether a sharded activation gets gathered depends on where the user's interventions are parked, so all ranks must decide identically or NCCL deadlocks. Only rank 0 answers the client; the shards' values are identical and dropped. Three things follow from that: - Requests go in two phases. Every rank proves it can *build* the block before any rank starts a forward, because deserializing is where a request usually dies and the last point one can fail safely. base.execute() grew a commit() hook for exactly that seam; the shards are released from it. - Cancelling is cooperative. Killing rank 0's thread mid-collective would strand the others in NCCL forever, so rank 0 broadcasts a byte at the root module's forward and every rank unwinds at the same iteration, leaving the group intact. Verified against a live stack: a 20s timeout stopped a generation and the replica served the next request unchanged, same shard pids, no restart. - Every rank is seeded identically per request. Sampling that diverges is a correctness bug, not an inconsistency: the ranks would go on to all-reduce activations computed from different tokens. nns.py holds the block-running half of a request, shared with the shards -- they must run it *identically* (the autocast dtype especially, or the ranks reduce different dtypes) and they have no use for the actor's Ray, boto3 and influx imports. Not yet: HOT<->WARM caching (restoring can reassign GPUs, which the group's fixed device mapping cannot follow), and the degree is pinned at 4 until the controller can size a deployment by what it shards into rather than by how many cards hold it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The controller now decides for itself whether a multi-GPU replica runs tensor-parallel. It asks nnsight how big the checkpoint is and how many ways it splits (see the matching nnsight commit), rounds the card count up to a workable degree, and routes to TPModelActor only when the model shards evenly into exactly the cards it was given. Everything else -- one card, no sharding plan, a degree that can't reach the count -- keeps the default actor, which spreads whole layers with accelerate. NDIF_TP_MODEL_ACTOR_CLASS points that choice somewhere else, including back at the ordinary actor to turn the feature off. Two accounting changes come with it: * A multi-GPU model is charged its *share* of each card, not all of it. Charging 100% of every card a model touched meant a replica using a third of four cards blocked all four -- fine when multi-GPU meant "too big to share", wrong now that a sharded 3B model spans four cards using 1.6GB each. * A replica whose actor sets CACHEABLE = False is evicted outright rather than demoted to WARM. A tensor-parallel group cannot be parked: every rank's device is fixed when its process starts, so restoring onto a different set of cards is not a thing that can happen. Demoting one anyway left the controller believing in a WARM replica while the actor held its GPUs. Three bugs this shook out, all found on a live 8xA100 stack and all with the same shape -- silent until the second model, or the second request: * A single-GPU model was being placed with device_map/max_memory, and transformers.pipeline puts a collapsed device map on cuda:0 whatever the map said. Invisible while every model gets card 0; the moment one doesn't, the actor refuses to start with "on cuda:0, outside the assigned set [2]". Per-share charging makes that the common case, so it surfaced immediately. Single-card loads now pass device= explicitly. * The shard processes never undid their per-request source-cache mutations, which rank 0 has always done. A second trace from the same file then parsed the *first* one's registered source, found no `with` at its line, and left the forward -- stranding every other rank in a collective until the execution timeout. Both sides now share one nns.block_scope, which is the point of that module. * A user error inside a traced block restarted the whole replica. Every rank runs the block, so every rank raises; reading that as a wedged group tore down four GPUs on every user-code bug, and the person who paid was whoever sent the next request. collect() now separates a shard that raised and went back to idle from one that never answered, and only a shard that raised *alone* -- the ranks genuinely diverging -- costs the replica. Also floors transformers at 5.15: below it, tensor parallelism serves a tied-embedding model logits tp_size times too wide. tests/test_placement.py is new and needs no server -- placement arithmetic and the shard-group settle verdict over synthetic objects, all three regressions above included. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The two-phase protocol exists so a payload that fails to deserialize is reported before any rank starts a forward -- deserialization is where a request most often dies, and it is the last point at which failing is still cheap. It did not deliver that. Every such failure restarted the replica, a full multi-GPU reload, and the person who paid was whoever sent the next request. Two defects composed. `prepare` sat one line above the `try` that owns `release`, so a ShardError escaped without standing down the shards that had already prepared -- they sat on a GO that was never coming until their hour-long idle timeout. And `release` produced no reply while `collect` unconditionally demanded one, so even the handled path -- rank 0 failing to build what the shards had built -- timed out and restarted. `release` bought nothing. Now: `prepare` is inside the try; the flag tracks whether any shard *heard about* the request rather than whether prepare succeeded, because prepare sends to every shard before collecting and a payload one rejects leaves the others parked; SKIP is acknowledged with IDLE, including at the top of the shard's loop, where a shard that rejected the payload has already returned to; and `_await_shards` does not collect from a request that never ran. A shard that will not confirm it stood down is still terminal -- that is the one version of "didn't run" that is not survivable. `collect` also charged its full timeout per shard rather than once for the group, making the real bound `timeout * (tp - 1)` -- 210s at degree 8 against a 30s constant chosen precisely because this runs on the actor's event loop, where a long wait stalls every other call into the actor including the cancel that might be trying to free it. It now works to a single deadline. The existing tests could not see any of this: they stub the group and check the verdict function, so they cover what `_await_shards` concludes and not the state machine feeding it. The three regressions fixed earlier on this branch were all verdict-shaped, which is why this one survived them. New tests drive the real `execute` against a scripted group; the fix was additionally verified against real shard processes running shard.py's loop, where a rank rejecting a payload now stands the group down in 0.00s and the next request runs immediately. docs/developing/checkpoint-description-proposal.md records the related interface question, and one live problem it turned up: the Hub calls behind placement have no timeout and run inside the controller's event loop. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`trusted` decides *where* a request runs -- in the model actor next to the weights, or in a runner process over a socket -- and nothing else. Two runs of one script must agree. They did not, in two compounding ways, and neither failed anything: the numbers were just different. The sandbox grew its own copy of the block executor before the shared one existed, and the copy had no autocast region, so a tensor the *block* made came back float32 untrusted and bfloat16 in-process. It now calls `execute_traced_block`, with the dtype shipped from the host because the runner has no model to ask. Fixing only that leaves the larger half. Under the sandbox the block runs in the runner but the model's **forward runs on the host**, driven over the socket -- so a bracket in the runner never touches the model's own arithmetic. Measured on gpt2: identical token ids, identical embeddings, diverging inside the first transformer block and ending at a relative difference of 6.5e-3 in the logits. The region is now one `request_dtype` used by all three execution paths, the host included, and the two paths are bit-identical. If you re-measure this, run the same path twice first: trusted against trusted is bit-exact, which is what makes a trusted-vs-untrusted difference mean anything. The runner also gets `block_scope`, which a fresh runner per request makes redundant today -- it is there so that stops being load-bearing. `resolve_dtype` learned to accept its own inverse: `str(torch.bfloat16)` is "torch.bfloat16", which it used to reject, and a caller shipping a dtype over a socket reaches for `str` long before a prefix strip. Two things this turned up, neither of them new: * `docs/developing/testing.md` told you to "set trusted=False in the trace's request envelope", which is not possible -- nnsight's RequestModel has no such field. `tests/conftest_untrusted.py` injects it, and the page now says how. 46 passed, 2 failed, 2 skipped through runner processes. * Those 2 are `TestRemoteGradients`: `.grad` raises OutOfOrderError over IPC. Confirmed pre-existing by A/B against the old code. It was not in the sandbox's deferred list; it is now, and "no autocast" -- which was listed -- is gone. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… to it Everything about where a replica goes came from one number -- the model's padded size -- and the only lever on it was `padding_factor`. So "give this model four cards" had to be said as a fudge factor computed backwards against the cluster's card size: the test config for a 6.4GB model at degree 4 was `padding_factor: 35.0`, correct only for 79GiB cards and wrong the moment the hardware changes. `DeploymentConfig` gains four overrides, each replacing one step of that derivation while the rest is still worked out: gpus place on exactly this many cards size_bytes the weights, measured -- skips the estimate padding_bias flat headroom, per model rather than per cluster max_tp largest sharding degree; 0 never places it tensor-parallel `size_bytes` is also the one placement input that needs no network: a deploy that names its own size goes through with the Hub unreachable, where an estimated one cannot be placed at all. A requested count is still checked against what the model can split into -- three cards on a model that shards eight ways is refused here, rather than at load after the cards are reserved and the weights read. That refusal now says *why*. `Candidate` carries a reason and the deploy error prefers it, because the generic "the cluster ran out of room" sends someone to look at the cluster when the problem is in their config. Plumbed through models.yaml and `ndif deploy` (`--gpus`, `--size-bytes`, `--padding-factor`, `--padding-bias`, `--max-tp`). Verified on 8xA100: `gpus: 4` gives a degree-4 tensor-parallel replica with default padding and no fudge factor; `gpus: 3` is refused with the reason; `gpus: 3, max_tp: 0` spreads it layer-by-layer instead. Also adds src/ndif/services/ray/tp/ARCHITECTURE.md -- the rank-0-actor topology, the two-sided load rendezvous, the two-phase request and why a stand-down has to be acknowledged -- and brings the docs back in line with this branch: sizing no longer builds a meta model, and a multi-GPU replica is charged its *share* of each card rather than 100% of every card it spans, which four pages still described. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Two defects in the placement overrides added last commit, plus a defensive rewrite of how the shard replies are read. **`max_tp` widened instead of capping.** `max_tp` returned the configured value without reference to the checkpoint, so `max_tp: 8` on a model whose weights divide two ways made 8 look like a workable degree: `node.evaluate`'s divisibility check passed, `actor_class` returned the tensor-parallel actor, and the group reserved eight cards and read the weights across them before transformers refused. That is the exact sequence the override's own guard exists to move earlier. It is now `min(override, limit)` when the checkpoint says a number, with a warning when it clamps. An override still wins where there is nothing to check against — an unreadable checkpoint, or one reporting no plan — because "supply a degree nnsight cannot work out" is half of what the field is for; that case warns too, since if it is wrong the failure lands at load. Verified against Qwen2.5-0.5B, which really does shard only two ways: configured `max_tp: 8, gpus: 4`, the controller clamps to 2 and the deploy is refused with "asked for 4 GPUs, but this model shards at most 2 ways" instead of taking four cards. **`ndif deploy -f models.yaml --gpus 4` silently dropped `--gpus`.** Only `--padding-factor` was threaded into `load_model_config`; the other four flags were accepted by click and hardcoded to None for file entries. Each now has a `default_*` parameter, so a flag applies to entries that don't set it and a value in the file still wins — the same rule `--pinned` and `--dtype` already follow. The test asserts the invariant rather than the instance: any CLI option with a matching `default_*` must be passed. **`collect`/`release` now wait on the whole group at once** (`select`) rather than each shard in turn with a shrinking timeout. Not a bug fix -- the verdicts are identical, which I checked across six timing patterns before writing this -- but the serialized form could hand `settimeout` exactly `0.0`, which is *non-blocking mode* rather than "no wait", and it made the outcome depend on the order shards happen to sit in a list. Neither is a property worth relying on. The new tests are the first to drive this protocol over real sockets; the previous ones stubbed `collect` and could only see the verdict, not the reading of it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…e fix is spelled right
The invariant is "the same script returns the same bytes trusted and untrusted".
It was false twice over, both times silently, and both times I found it by
measuring by hand. What I then committed in its place was six
`inspect.getsource` substring assertions — which pass whether or not the numbers
agree, and would have passed against the broken code if `request_dtype` had been
entered around the wrong scope, entered and immediately exited, or entered with a
dtype that makes it a no-op.
`tests/test_sandbox_conformance.py` runs one trace both ways against a live
server and compares the saved values. It leads with the control — the same path
twice, bit-exact — because without that a difference between the paths could just
be a model that isn't deterministic, and the comparison would mean nothing.
Confirmed it catches what it is for by pointing it at the pre-fix sandbox:
made is torch.bfloat16 trusted and torch.float32 untrusted
-- the two paths are not applying the same autocast
hidden differs between the paths by at most 2.500e-01
which is exactly the pair of bugs: the runner's missing region, and the host's
missing region around the forward it drives.
The structural tests stay, retitled to say what they are. They catch the *shape*
that let the two implementations drift and nothing about the numbers, and the
class docstring now says so, because believing otherwise is how the divergence
survived in the first place. `test_there_is_one_definition_of_the_region` also
counted `torch.autocast` occurrences tree-wide and would have broken on the first
unrelated use; it now asks whether any execution path builds its own region
instead of calling the shared one, which is the property.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`NDIF_TP_MODEL_ACTOR_CLASS` unset now means the feature is **off**, not "use the built-in actor". It was a default; it is a switch. Off is not merely "route multi-GPU replicas elsewhere". No sharding degree is worked out, so nothing reaches the Hub for one; no GPU count is rounded up to a degree a model divides into evenly; a per-model `max_tp` does nothing; and no replica is routed to the TP actor. A cluster that has not named the actor that serves a sharded model has not opted in, and shouldn't be handed one behind its back — a tensor-parallel replica cannot be cached, is not sandboxed, and needs transformers >= 5.15 to shard correctly. Those are choices, not details. An empty value counts as unset, because clearing a variable in a compose file leaves "" behind and that must not resolve to an actor class named "". Verified on 8xA100 with the config that produced a degree-4 tensor-parallel replica an hour ago — `gpus: 4, max_tp: 8` on Llama-3.2-3B. Unset: the ordinary actor, zero shard processes. Set: `TPModelActor` and three shards. The suite passes either way. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Phase 1 of putting untrusted code on a tensor-parallel model. Nothing about behaviour changes; this is the seam that has to exist first. `SandboxModelDeployment` did two jobs. It was the Ray actor — pool, request lifecycle, statuses, errors — and it was also the *host* of a sandboxed run: `MediatorProxy` per worker, the interleaved forward, the control events a worker can park on. Only the first is about being an actor. The second is what any process holding a model and a socket has to do, and a tensor-parallel shard is exactly that: a model, a socket, and no actor around it. `SandboxDriver` (`sandbox/driver.py`) is the second job with the first removed. It takes a model, a dtype, and somewhere to put a runner's stdout — no Ray, no request type, no pool. `SandboxModelDeployment` drops from 424 lines to 135 and now reads as what it is: acquire a runner, hand it to a driver, translate the result for the client. The one thing the driver cannot know is where a log line goes, so that arrives as `on_log`. Also folds three near-identical socket wrappers into one. The sandbox host's `Connection`, the runner's, and the tensor-parallel `Channel` each reimplemented framing, timeouts and close over the same `protocol.py`. They are now one `Channel` with the two direction-specific codecs as overrides — the host sends a single encoded value and the runner replies with an event name plus values and kwargs, and that asymmetry is deliberate, so it stays. Net -286 lines. Verified on 8xA100 that nothing moved: the untrusted suite is 46 passed / 2 failed / 2 skipped, the same two pre-existing `.grad` failures as before the refactor; the trusted-vs-untrusted conformance suite is still bit-exact; the full suite 53 passed. docs/developing/sandboxed-tensor-parallel-proposal.md is the plan this serves. The short version of why the seam is worth having on its own: a rank cannot act as a sandbox host until the host logic is separable from the Ray actor, and that is true whichever topology the sandboxed tensor-parallel design ends up with. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… block Phase 2 of putting untrusted code on a tensor-parallel model. Two things that are wrong today and become load-bearing the moment a rank's block moves to a runner. **`seed_ranks` seeds the wrong process.** Under tensor parallelism every rank must draw the same numbers within a request — ranks that sample differently generate different tokens, and the model's own all-reduces then sum activations computed from different sequences, so the answer is wrong on every rank rather than merely inconsistent. That seed is applied in the *rank* process. Once the block runs in a sandbox runner, the RNG it draws from is out there, and seeding the rank seeds nothing that matters. The seed now rides to the runner beside `dtype` and is applied by the shared executor, immediately before the block rather than before deserializing it — what has to match across processes is the state the *user's* code draws from. `seed_ranks` becomes an alias of one `nns.seed_block`, so a rank and a runner cannot drift. `SandboxModelDeployment.block_seed()` returns None, which is the right answer for one process: two identical requests are *meant* to draw differently, and seeding every one would quietly turn repeated sampling into repeated identical samples. A deployment whose block runs in several processes at once overrides it. **`PYTHONHASHSEED` was pinned nowhere.** Neither `rank_env` nor the runner spawn set it, so every process gets randomized string hashing and `set`/`dict` iteration order differs between them. A block that iterates a set of module names, or breaks a tie by iteration order, already takes a different path on each rank today — invisible in one process, a hang across a group. Verified in the container: two unpinned processes hash the same string differently, two pinned ones agree. Now pinned in both spawn paths. Neither is reachable through today's shipped paths, which is why they have gone unnoticed: a single-process request cannot diverge from itself, and the tensor-parallel path runs the block in the rank where the seed is applied. Both become live in phase 4. Verified on 8xA100 that the widened wire tuple changes nothing: untrusted suite 46 passed / 2 failed / 2 skipped (the same two pre-existing `.grad` failures), conformance still bit-exact, full suite 53 passed. The runner unpacks the seed with `*rest` so it does not require a host that sends one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Phase 3 of putting untrusted code on a tensor-parallel model. Nothing uses this yet; it is the only genuinely new logic the design needs, and it is testable without a model, a GPU or a server, so it is worth having settled before anything depends on it. `Fanout` presents `send`/`recv` over several peers instead of one: send reaches all of them, recv waits for all of them, checks they are asking the same thing, and returns one answer. Two properties fall out of that, and they are why this is a type rather than a loop at the call site. **A worker is served once per serve, not once per rank.** The runner holds one set of workers, and resuming one per rank would run the user's block N times over. Because `recv` collapses N arrivals into a single message, the loop that drives the workers — `while True: message = connection.recv()` — is unchanged. That is the whole reason a sandboxed tensor-parallel request can be the same code as a sandboxed single-GPU one with a different number of peers, and there is a test that drives a pump-shaped loop over three scripted ranks and asserts the worker ran twice for two locations rather than six times. **Ranks that stop agreeing are reported, not waited on.** A peer resuming a worker while another finishes has already taken a different path, and the next collective either hangs or combines tensors computed from different things. `RanksDiverged` names which peer disagreed. The wait is bounded for the same reason — a genuinely diverged rank never arrives, and an execution timeout with no cause attached is a worse way to find out. What must match is pluggable, defaulting to the event name; a caller that knows the protocol can compare the control fields too. Writing the tests found that I had the codec pairing backwards: the runner *receives* what a host sends (`encode`/`decode`) and *sends* in the packed runner→host shape, and the first version of both the test helpers and my assumptions had those swapped. The asymmetry is deliberate and documented, and a Fanout has to sit correctly on one side of it — worth having learned from a 2-second test rather than from a wedged group. 16 tests, and they carry no `boto3` guard unlike the suites beside them, because `protocol.py` is pure transport: they collect and pass in a client-only environment too, which is where someone debugging the wire will be. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Phase 4. A tensor-parallel rank can now serve an untrusted request the way the single-GPU actor does — driving the forward and answering reads over a socket — instead of running the user's block itself. Nothing selects this path yet; that is phase 5. The pieces were built in the previous three commits and this joins them. A shard handles a new `SANDBOX` message by connecting to the runner rank 0 names, building a `SandboxDriver` (phase 1) and pumping it. There is no block to deserialize on a shard any more, so the two-phase protocol keeps its shape but changes meaning: `READY` becomes "the environment is applied and I can reach the runner", which is still the last point before any collective and so still the safe place to fail. The runner learns at launch how many hosts drive one request. Above one it wraps them in a `Fanout` (phase 3), so a worker is served once for the whole group and every reply reaches all of them; at one it is the plain connection it always was. The shards seed their own RNG as before — the *model's* sampling still happens in the rank even when the block does not — while the block's RNG is seeded in the runner from the same number (phase 2). Two things this shook out: `spawn` learns a runner is up by connecting to its socket and closing again. With one peer that probe merely caused a failed exchange and a retry. With several it would have taken one of the group's places, and the runner would have waited forever for a host that had already gone. The accept loop now checks whether a peer has already hung up — without consuming anything a live one sent — and there is a test that connects a probe before the real hosts. `Writer` forwarded stdout through a `Connection`-only helper, which a `Fanout` does not have. It now sends the event directly, so one writer serves one host or a group. Also stops `test_sandbox_conformance.py` running under `-p conftest_untrusted`. The plugin forces every request untrusted by patching the same method that suite toggles, so together they compare untrusted against untrusted — passing the equality tests for the wrong reason. It cost me a wrong "4 failed" reading before I noticed; it now fails loudly instead. 20 fanout tests including three that drive a real runner subprocess as a group. On 8xA100 the single-GPU sandbox is unchanged: untrusted 46 passed / 2 failed / 2 skipped (the same two `.grad` failures), conformance bit-exact, full suite 73. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`SandboxedTPModelDeployment` forks per request, exactly as the single-GPU sandbox actor does: a trusted block runs on every rank in-process, an untrusted one runs once in a runner that every rank is a host to. Keeping it one actor rather than two axes is what keeps the controller's choice simple -- it picks by how the *weights* are placed, and the actor decides per request where the *code* runs. The alternative was a four-way matrix of classes for two independent booleans. The untrusted path needs no decision channel, which is the part worth saying because it looks like it should. Every rank keeps its own mirror of where the workers are parked, so each answers "is anyone waiting here?" locally and the ranks agree about gathering with nothing sent. What the runner adds is a barrier: it serves each worker once for the group rather than once per rank. Two orderings in `_sandboxed` are load-bearing. Rank 0 connects before the shards are told where the runner is, because the runner takes its first connection as the one that sends the payload. And `commit()` -- go() plus the abort arm -- lands after every shard has answered READY, so a shard that cannot reach the runner fails while nothing is in a collective. `NDIF_TP_MODEL_ACTOR_CLASS` already takes a dotted path, so selecting this is an operator's choice with no new plumbing.
`kill` is gated on `execution_ident` being set -- it is how the actor knows a request is in flight at all. The trusted path gets it from the base's `execute`; the untrusted one never calls that, so a cancel or a preempt was silently dropped and the block ran to completion. Found by reading the single-GPU sandbox actor, which sets it explicitly for exactly this reason. Nothing failed: the request completes and returns the right answer, and only a client that asked to stop it would notice.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`wait_for_replica_ready` caught `RayActorError`, which is the parent of both `ActorUnavailableError` (the actor is restarting -- wait) and `ActorDiedError` (its constructor raised -- stop). Catching the parent swallowed the second, and since `max_restarts=-1` respawns the actor to raise the identical error again, a permanent failure was indistinguishable from a slow start. The poll ran out its 300s and reported "initialization timed out". The reason was in hand the whole time: `ActorDiedError` carries the creation task's traceback, and `deploy` already had an `except Exception` branch that prints it. It never fired because the exception was discarded two frames earlier. So catch only the two states that mean "not yet", after the API's `Replica.wait` -- which had this right -- and let everything else out. Deploying gpt2 forced tensor-parallel now fails in 21 seconds with the `UnshardableCheckpoint` message in the terminal, rather than in five minutes with a deadline. The timeout goes with it. A deadline cannot tell slow from broken either: it reported both as a timeout, which is right for neither -- a genuinely slow load across many GPUs was cut off, and a deterministic failure was described by how long it had been polled rather than by what went wrong. Now that the cause escapes, waiting indefinitely is only waiting for something that will happen. Verified on hardware both ways: a restart polls through the respawn and succeeds (19.9s), and the refusal propagates with its message.
Every shard seeds itself before a sandboxed request (shard.py's SANDBOX branch) and rank 0 did not. The block runs in the runner, but the *model* still runs in each rank's own process, so its sampling draws from rank 0's unseeded RNG while every other rank draws from a seeded one. The consequence is the one `seed_block` exists to prevent: ranks sample different tokens, then all-reduce activations computed from different sequences, so the answer is wrong on every rank rather than merely inconsistent -- and a stopping criterion that diverges hangs the group in NCCL instead. Many checkpoints ship `do_sample: true`, so it reaches requests that never asked to sample. Invisible because every file looked right on its own: the shards seed on both paths, and rank 0 seeds on the trusted one. Only the pairing was wrong, so the tests pin the pairing rather than either half -- and both fail with the line removed. Found by an auditing pass over the TP/sandbox code. Also from the same pass, all verified: drop `run_traced_block`, dead since the `commit()` seam split its two halves; fix its docstring, which described a single-process actor calling it. Correct three comments that state a sharded replica "is not sandboxed", false since SandboxedTPModelActor. Correct the sandbox architecture doc, which said `Fanout` was unused when it is the barrier the whole shared-runner design rests on, and drop a duplicated `protocol.py` row from its table.
`Fanout.recv` waits for every peer, compares signatures, and returns peer 0's message. Every other peer's payload was serialized, sent, rebuilt and then dropped -- and under tensor parallelism those payloads are whole activations, identical on every rank because the gather already made them whole. At degree 8 that is seven eighths of the host-to-runner leg doing nothing, and the cost is worse than bandwidth: each discarded payload is deserialized, allocating a tensor, before it is discarded. A shard now connects with a `FollowerConnection`, which sends the signature alone. Every shard is a follower by construction and rank 0 is the actor, so there is no rank to plumb through. The barrier is untouched -- still one message per event per peer, in the same order, so a peer resuming a worker while another finishes is caught exactly as before. What is given up is the *option* of comparing payloads across ranks, which was never taken: the signature has always defaulted to the event name, and comparing whole tensors at every location would cost more than the interleave it guards. `_event_of` becomes `signature_of` -- it is what the barrier calls a signature, and it is now part of how a follower builds its message rather than a private helper. Verified on the sharded deployment: trusted and untrusted still bit-exact, the nine-check exercise still passes with the sharded model's value pinned, and the single-GPU suites are unchanged (55 passed; 48 passed with the two known .grad failures).
`torch.load` takes a `map_location`; a plain pickler does not. Moving the event codec onto `torch.save`/`torch.load` lets a host say where the tensors in a message should land *as they are rebuilt*, which is the right place for the policy: relocating afterwards -- what this did until now -- allocates on the sender's card first, and under tensor parallelism that card is another rank's memory budget. So `pump` sets the connection's map_location once, when a host takes charge of a runner, and the walk over every arriving message goes away. The relocation in `_assemble` stays: the batcher and tokenizer run host-side, so those tensors are made locally and never cross the wire. The runner leaves map_location None on purpose. Its block is meant to compute on GPU, and pulling activations to the CPU would quietly move the user's arithmetic there and stop matching the in-process path -- which the bit-exactness check between trusted and untrusted would have caught, and does still pass. cloudpickle goes with it. The request payload rides as raw bytes through the codec's str/bytes fast path, already serialized by nnsight, so cloudpickle only ever wrapped the per-event values -- tensors, parks, name lists, config dicts. The case that loses is a swap value whose class is defined inside the user's block, which is now unsupported. An unknown wire tag raises instead of falling through to a pickler, so two ends built from different revisions say so rather than misreading each other's bytes. Verified on hardware: trusted and untrusted still bit-exact on the sharded model (which is also what proves the runner still gets GPU tensors), nine-check exercise passes with the sharded value pinned, single-GPU suites unchanged at 55 passed and 48 passed with the two known .grad failures.
The queue provisions a replacement with a bare `DeploymentConfig(replicas=1, trusted=...)`. It has no idea how the model was deployed, and shouldn't -- but nothing else remembered either, so a model an operator placed tensor-parallel came back on the default single-GPU actor under the same model key. Same model key, different math, nothing saying the model is no longer sharded: measured live, the sharded replica answered 424.7618 and its replacement 424.2299 for the same block. Sharding is only the loudest case. `dtype` is the same hazard with nothing to notice it by -- a model deployed float32 would return as bfloat16. So the controller remembers the placement fields (`DeploymentConfig.STICKY`) per model key and fills in whatever a later deploy leaves unset. An explicit value always wins, and becomes what is remembered from then on, so redeploying with new settings still changes them. `replicas` is excluded because it is additive per call, and `trusted` because it belongs to the request that triggered the deploy. Not persisted across a controller restart: the first deploy after one re-establishes whatever it is told, and inventing a durable store for this would be a much larger change than the problem needs. Verified live: with the sharded replica evicted, a request auto-provisioned replica a76b2 as SandboxedTPModelActor across 2 GPUs and answered 424.7618 -- the sharded value -- where before it came back single-GPU.
Hosting a runner was written twice -- once in the single-GPU actor and once in the tensor-parallel one, which copied it. The copies drifted exactly once and it cost a correctness bug: the tensor-parallel copy never seeded rank 0's own RNG, so the model sampled a different token there than on every shard it was about to all-reduce with. `SandboxHost` is what both copies agreed on: the pool, the per-request runner, the acquire/send/pump body, and the error and cleanup paths. Each actor is left with what is genuinely its own -- the sandboxed group keeps four methods, the single-GPU actor two. The body takes two hooks because that is exactly where the two differ: a group tells its shards where the runner is *before* the block is sent, and releases them into the forward *after*. Both moments are load-bearing and neither exists for one host. `interrupt` stays with each class, deliberately in opposite orders: the single-GPU actor stops the runner then lets the base kill the thread, while a group cannot -- killing rank 0 mid-collective strands the rest, so it asks every rank to stop at a shared checkpoint first. `block_seed` goes: it was an override hook with no override, whose docstring pointed at a subclass that did not implement it. The seed is now a parameter of the shared body, which is where the contract belongs -- one process wants None, several running one block want the same number. The tests move with it. The ordering invariants used to be asserted against two copies of the same body; they are now asserted once, against the body that carries them. Verified on hardware: trusted and untrusted bit-exact on the sharded model, nine-check exercise passing with the sharded value pinned, single-GPU suites unchanged at 55 passed and 48 passed with the two known .grad failures.
…ment `spawn` copied `os.environ` wholesale into every runner. The runner exists to execute other people's Python, and the actor's environment is the operator's: measured on a live deployment it carried `HF_TOKEN` and `NDIF_INFLUX_TOKEN`, and a production one would add object-store credentials and `NDIF_POSTGRES_URL`. `os.environ` is the first place a block would look. So `runner_env()` builds the environment from `RUNNER_ENV` instead -- the loader and locale basics, `CUDA_VISIBLE_DEVICES` (the block computes on GPU and its device numbering has to match the host it drives), cache locations so a block that touches the Hub finds the shared cache, and the thread-count knobs an operator tuned. An allowlist rather than a denylist because a denylist is wrong by construction here: the next credential added to the actor's environment would be inherited by default. It also drops `RANK`, `LOCAL_RANK`, `WORLD_SIZE`, `MASTER_ADDR` and `MASTER_PORT`, which closes something an audit raised separately: rank 0 sets those on itself to join the process group, and a runner spawned from it started life claiming to be rank 0 of a group it is not in -- exactly what accelerate and transformers read to decide they are in a distributed launch. Verified by asking a block: an untrusted trace on the sharded replica reads none of the five credentials and neither set of rank variables, while CUDA_VISIBLE_DEVICES and PATH are still there. Trusted and untrusted remain bit-exact, the nine-check exercise passes, and the single-GPU suites are unchanged at 55 passed and 48 passed with the two known .grad failures. This does not make the sandbox a security boundary -- the runner is still an ordinary process with no hardening -- but it stops the most direct way out of it.
Remembering placement fields per model key fixed the queue's problem by changing everybody's. The CLI sends an explicit `None` for any field a spec omits, and `None` was exactly the signal to reuse what was remembered -- so `ndif deploy <model>` stopped meaning "deploy with the controller's defaults" and started meaning "deploy however that model is currently served". Worse, a field could then only be changed, never cleared: redeploying without a timeout kept the old one. One rule for every caller is simpler, but the caller that needs the behaviour is the queue, which provisions a replacement with a bare config because it has no idea how the model was deployed. An operator typing a deploy command is not in that position and should get what they typed. This reverts b220695. The bug it fixed is real and comes back: a replacement replica for a tensor-parallel model is provisioned on the default single-GPU actor, answers under the same model key, and says nothing about no longer being sharded (424.7618 vs 424.2299 on the same block). `dtype` has the same hole. Whatever replaces this has to be scoped to the provisioning path rather than applied to every deploy.
Growing a model should add *more of what is there*, not a second opinion about how the model is served. The autoscaler provisioned with a bare config, so a busy tensor-parallel model grew a single-GPU replica: same model key, different numbers (424.7618 against 424.2299 on the same block), and nothing telling the caller which replica answered. `scale` fills a config's unset fields from a replica already serving that model and adds `n` more. Additive like `deploy --replicas`; the difference is only where the unspecified settings come from -- `deploy` uses the controller's defaults, which is right when you are saying how a model should be served, and `scale` copies what is running, which is right when you are saying more of that. The live cluster is the source of truth rather than a remembered config. Nothing to persist, nothing to expire, and no way for it to change what an explicit value means: a field the caller set always wins, and is also what makes a replica "matching" enough to copy the rest from. This replaces the reverted b220695, which got the behaviour right and the scope wrong -- it applied to every deploy, so `ndif deploy` silently stopped meaning what it said. `Replica.provision` routes through it, which is the point: that call knows nothing about how a model is served and has no business deciding. Cold start is deliberately unchanged. With no live replica there is nothing to copy, so the evaluator decides as it always has -- nothing in the cluster claims the model is served any particular way, and a durable answer to that belongs in a config file. Size is deliberately not copied. A Deployment's size_bytes is the evaluator's output, already padded, while the config field means the model's own weights with padding applied on top; copying one into the other pads twice. Caught on hardware, where a scaled 2-GPU replica charged 70.9 GB per card against the original's 33.6 GB. Verified live: `ndif scale meta-llama/Llama-3.2-1B -n 1` against a sharded replica produced a second one identical in actor class, GPU count, per-card charge and timeout, and six requests alternating trusted/untrusted across both returned 424.7618 every time.
Corrects what became false, rather than rewriting pages: * the sandbox codec is torch, not cloudpickle, and the point of the change is that `torch.load` takes a `map_location` so the receiver names the device; * activations no longer "assume the runner sees the same device" -- that assumption is exactly what broke every untrusted request on a sharded model; * both sides autocast now, so an untrusted request returns the same numbers as the trusted one; the page still said the runner did not; * the CLI's readiness wait has no deadline, so five places promising `initialization timed out` after 300s were wrong in the direction that matters: a constructor that raised now reaches the operator as its own error; * the payload is a 4-tuple, in the message catalog that calls itself authoritative and in two docstrings that restate it. `sandbox-internals.md` had twenty `model.py:NNN` citations left behind by the driver extraction, most past the end of a file that is now half the size. They name modules and symbols instead: line numbers that rot on the next refactor buy less than a name that does not, and the page's own frontmatter had never listed `driver.py` at all. The intro now says what the split is for -- `driver.py` answers the runner, `model.py` answers the client, which is what lets a tensor-parallel shard be a host. Documents `ndif scale`, including the part worth knowing: it is additive like `deploy --replicas`, and differs only in where unspecified settings come from.
`ndif deploy meta-llama/Llama-3.3-70B --dtype nf4` now holds the weights
4-bit. Nothing else about the deployment changes and nothing client-side
does either: module paths and activations are the same as an unquantized
replica, so a trace written against one works against the other.
Almost all of the plumbing was already there -- `dtype` travels as a string
from the CLI through the config, the evaluator and the actor -- so what this
adds is the one place it could not stay a string. A quantization is not a
`torch.dtype`, and the actor had exactly one field for both jobs:
dtype_name what the weights are HELD as. Stays a name, because that is
what nnsight's loader needs to build a quantizer config.
dtype what the model COMPUTES in. A real torch.dtype, which is what
user execution autocasts to and what the sandbox runner is
told -- neither cares how the weights are stored.
Getting that backwards is quiet in both directions: load with the resolved
dtype and the quantization is silently dropped, putting a full-width model
on an allocation sized for a quarter of it; autocast with the name and there
is no dtype to autocast to.
`resolve_dtype` reads nnsight's table rather than keeping a second list, so
a format added there is understood here without either drifting. Rank 0 and
the shards are handed the same name for the same reason -- ranks that held
their weights differently would all-reduce mismatched values -- though
quantization with tensor parallelism is untested and documented as such.
Documented but not fixed: the size estimate runs low by more than the
default padding absorbs (Llama-3.2-1B nf4 estimates 0.62 GB against 1.07 GB
really allocated), because the format leaves embeddings and the LM head in
16 bits. Deploy a quantized model with a measured --size-bytes until the
estimate accounts for that.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The section said the combination was untested. It has since been run: Llama-3.3-70B-Instruct at nf4 over 4 A100s on hakone, deployed with `--dtype nf4 --gpus 4`. A remote trace reads layers[40].mlp.gate_proj.output at its full 28672 rather than one rank's 7168, so nnsight's gather works through the quantization. 43.3 GB across the four cards against ~141 GB for bfloat16. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Give the sandbox runner a meta build of the model it is driving, so a payload's Module:/Tokenizer/Pipeline ids resolve to real weightless objects in that process instead of to None. A block can tokenize, walk the tree and read shapes locally; every activation still crosses the socket, because the weights are only ever the host's. Reconciled with what 0.8 has done since the branch was cut, and extended where the merge exposed a gap: * `spawn`/`Pool` take `model_key` rather than the branch's `runner_args` list -- 0.8 already gave the runner a positional `peers`, and the pool is now opened by `SandboxHost.open_pool`, shared with the tensor-parallel actor, so both hosts get meta models from one line. * The runner is also told the actor's `trust_remote_code`. Without it a checkpoint that defines its architecture in its own repo has no config to read, and the meta build fails for exactly the models most likely to need it. * A failed build is reported and ignored rather than raised. It would otherwise kill the process before it binds, which `Pool.refill` swallows on its warm thread -- one unbuildable checkpoint would have presented as every request waiting out `acquire`'s 30s timeout with nothing in the log. * The request's `env` rides in the payload and is applied to the runner's meta model, mirroring what the actor does to its loaded one. A PEFT adapter prefixes every path with `base_model.model`, so without this the map would describe the unadapted tree and every id in an adapted request would miss it. The adapter's weights land on meta parameters, which is what is wanted -- the paths are the point. Best-effort, like the build. * `Remotable.from_model_key`, not `HuggingFaceModel`'s: the key names its own wrapper class, so this is right for a non-HuggingFace one too. Measured on gpt2: a runner's spawn goes 2.2s -> 4.1s and its idle footprint 423MB -> 765MB (PSS), so the default pool of 7 costs ~5.4GB per model actor rather than ~2.9GB. Most of that is transformers' modeling classes, the tokenizer and the meta pipeline, not the tree, so a much larger checkpoint adds far less than proportionally. Documented where the pool size is justified. Verified by driving a real runner against a loaded gpt2: the block's tokenization matches a local one, a meta module's weight shape reads without a round trip, activations still come from the host, a PEFT request resolves adapter paths in the runner, and a runner given no model key behaves exactly as it did before. Co-Authored-By: Claude Opus 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
2 participants
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.
Brings the 0.8 rewrite onto this repo as a single replacement commit against
dev.It is a big diff (310 files, +30.7k/−28.5k) but not a from-scratch rewrite. The parts that were working carry over; four areas change substantially, and one of them changes the security model. There's a suggested reading order at the bottom.
What carries over
Worth stating up front so the diff size isn't misleading:
controller/cluster/{cluster,node,deployment,evaluator}.pykeep their responsibilities — sizing a model on the meta device, picking a node/GPU, HOT/WARM/COLD levels, pinning, eviction, the minimum-deployment-time rule.actor_class.options(lifetime="detached", ...)resolved by name in theNDIFnamespace. Neither the old server nor this one uses Ray Serve.backend/+frontend/(Vue SPA) +jobs/(monitor, reconcile crons).What changes
1. How untrusted code runs — the security model is inverted
The previous server ran a user's traced block in-process in the model actor, contained by a six-layer in-process
Protector(ray/nn/security/): import interception, a module/builtin whitelist (whitelist.yaml), attribute guards, an audit hook, restrictedcompile/exec, and protected model/tokenizer objects. That entire directory is deleted.In its place, untrusted blocks run in a separate runner process — one fresh process per request, stopped afterward — with nnsight's interleaver split across a Unix socket. The model never leaves the actor; only the block moves. Worker greenlets live in the runner, while the parent side of each mediator (occurrence counting, the
tracer.iterpin, read/swap matching) stays on the host as aMediatorProxythat reuses stockMediator.handleunchanged.Two things to be clear about:
request.trustedis new. The old server had no such concept — everything went through the Protector. Now the flag, stamped at ingress from the API key, decides in-process vs runner. With auth off it defaults toTruewhen the client doesn't specify, so a zero-config stack runs user code in-process; a client can sendtrusted: falseto force the runner path.2. Client transport
Socket.IO with a Redis-backed cross-process manager (
common/providers/socketio.py,cli/lib/session.py) is replaced by a plain FastAPI websocket over Redis pub/sub. The client subscribes before it gets itssession_id, so no status can be published before someone is listening.3. Telemetry
OpenTelemetry tracing is removed — the collector, Tempo, the Tempo datasource and
common/tracing/all go. What remains is consolidated: logs to Loki, NDIF's own metrics to InfluxDB, Ray's to Prometheus, and nine provisioned Grafana dashboards (up from four).common/flattenslogging/,metrics/,tracing/intologging_setup.py,metrics.py,telemetry.py, and gains aredis/package for the coalesced status/env caches and the CLI event stream.4. Packaging and dev loop
One image, service selected at runtime by
NDIF_SERVICE, with thendifCLI as the entrypoint.justfile+ a pinnedrequirements.txtreplace theMakefile+uv.lock. nnsight is an ordinary dependency again rather than vendored —just up/just tabind-mount a local editable checkout over the image's copy viadocker/docker-compose.nnsight.ymlwhen one resolves, so client-side changes don't need a rebuild.5. Documentation
CLAUDE.mdis an agent-facing router over a new 61-pagedocs/tree — concepts, operating, runbooks, developing, reference, errors, gotchas. Pages carry frontmatter, cite sourcefile:line, and were checked for link and citation integrity. This replaces the single 2,774-lineNDIF.md.Breaking configuration changes
NDIF_BROKER_URLNDIF_REDIS_URL50018001270189000NDIF_RAY_SERVE_PORTNDIF_RAY_METRICS_PORT(it is Ray's--metrics-export-port; the old name implied Serve, which is unused)6379(collided with Redis)6385everywhereNDIF_MODEL_IMPORT_PATH,NDIF_CONTROLLER_IMPORT_PATH(resolving import paths for the model actor / controller classes)Everything is env-driven with a working single-host default, so a bare
just upstill comes up end to end.Dropped here, to be re-added deliberately
.github/workflows (image build, PyPI publish) — this branch intentionally has no CI. Re-add before merging anywhere that depends on them..env.example— it described the previous variable scheme and would reintroduce the drift above.docs/reference/env-vars.mddocuments every variable with its default and the line that reads it.NDIF.md(superseded bydocs/),telemetry/,scripts/,uv.lock,Makefile,.python-version.Known gaps (deliberately left, marked in code)
# TODOinupdate_nodes.tracer.barrier()andepropertywrite-back transforms are unsupported on the sandbox path (a barrier park raises; a transform is silently dropped).# TODOat both sites.# TODOfor a configurable limit.padding_factordoesn't survive anndif export→deploy -fround-trip; it's a deploy-time sizing input never stored on the deployment. Everything else does.nnsightis unpinned inrequirements.txt— pin it at the next release.How to review this
Most of the diff is documentation and deleted files. Suggested order:
CLAUDE.md, thendocs/concepts/request-lifecycle.md— one request end to end. Everything else is a zoom into one of its hops.src/ndif/services/ray/sandbox/— the genuinely new subsystem. Read itsARCHITECTURE.mdfirst;model.pyholds the trusted/untrusted fork and the host-side proxy,nns.pythe runner side.src/ndif/services/api/auth.py— wheretrustedis stamped; short, and it decides everything in (1).docs/developing/architecture-overview.md— process map, concurrency model, where state lives.docs/pages,frontend/dist/(committed build output), and the bulk deletions undersrc/ndif/services/ray/nn/.Trying it
The first remote trace of a model deploys it on demand — no explicit
ndif deployneeded.There is no CI on this branch, so the test story is "bring the stack up, run pytest." Note that a default stack runs everything trusted, so the sandbox path isn't exercised unless you send
trusted: false;docs/developing/testing.mdcovers that.