Skip to content

placement, model, gpu, gpu-gates: a Machine's host and unified pool live in Machine::nodes, this process's node first - #94

Merged
midagedev merged 2 commits into
midagedev:mainfrom
juliankang4:dgx-spark-nodes0
Oct 13, 2026
Merged

midagedev merged 2 commits into
midagedev:mainfrom
juliankang4:dgx-spark-nodes0

Conversation

@juliankang4

@juliankang4 juliankang4 commented Oct 13, 2026 •

Copy link
Copy Markdown
Collaborator

What and why

Item 1 of P5's placement half in docs/contrib/dgx-spark.md §8.2: a Machine lists its nodes, each with its own host and unified pool, and this process's node comes first. Every plan today reads nodes[0], so no plan changes. Part of #13 (P5, agreed in #13 not to wait for S10).

Two commits, so each one reads on its own:

  1. be407ec1 adds Machine::host, host_mut, unified and unified_mut, and moves every read and write of the two fields to them. The fields stay where they are.
  2. 7395c7dc moves the fields into Machine::nodes: Vec<Node> (Node { host, unified }). The accessors read nodes[0] and panic by name on a machine with no node. Every Machine literal now carries nodes: vec![Node { .. }].

The node plans (the next PR, #95) plan each node of Machine::nodes against its own pool, which is why this one goes first.

Rebased onto main at bd29e9c9, which carries the mac-remote self-test fix (#113). That rebase had no conflicts, and git range-diff shows both commits unchanged: 203cf8e5 is now be407ec1 and 83205a05 is now 7395c7dc. The runs under "What I ran" are on the commits before it. The rebase before that, onto d24cb1a4 past 0.3.2, had conflicts only in the second commit, in the test use lists of crates/gpu-gates/src/bind.rs and the glm5next and qwen35moe place.rs, where 0.3.2 changed the imports next to the Node this PR adds. The change itself is the same.

Code shape

  • No second copy. The accessors are the single owner of "this process's node"; no call site indexes nodes[0] itself.
  • Model code holds only architecture facts. The model place.rs files change only their Machine literals and field reads.
  • Dead code is gone.
  • No silent failure. A machine with no node panics by name in the accessor.
  • Comments state what is true now.
  • No relaxed gate.
  • cargo fmt and cargo clippy are clean on what I touched; the lint count did not go up.

Duplication found or removed (path:line), or "none": none.

What I ran

On the Mac, at 83205a05 (and tools/mac-check.sh check at 203cf8e5 alone):

(each step's line cut after its time; the lint step counts 41 warnings, the ratchet's 41)

$ tools/mac-check.sh check            # at 203cf8e5
mac-check: check rc 0; ... 25 bloomery crates checked or compiled
$ just mac-static upstream/main
mac-static: fmt-check rc 0
mac-static: lint rc 0
mac-static: combos rc 0                (20 shapes run, 0 red)
mac-static: check-arch, check-comments, check-defaults, check-levers, check-loads, check-ptx-names,
            check-records, check-recipes, check-rustflags, check-unsafe, check-waits: rc 0
mac-static: ok
$ just mac-test
mac-check: test rc 0; 12 crates ...; 1139 passed, 0 failed, 177 ignored in 45 test binaries

On one DGX Spark (aarch64), in the dev container, at 83205a05:

$ just contrib-check --tier t1
total=96 red=1

The one red step is check-recipes: main's tools/mac-remote.sh --self-test (from d24cb1a4) fails 4 of its lock checks on an aarch64 Linux host. Those checks stub uname -s as Darwin but read the host's own uname -m, which is aarch64 here, so the preflight stops at missing: a macOS host (this is Darwin aarch64). This PR does not touch tools/, and every other step is green. #113, now on main under this PR, fixes that self-test. Before the rebase onto d24cb1a4, at 27760114 on main at 9ab442ca, the same T1 had check-recipes green; its one red was check-levers, on main's BLOOMERY_MAC_REMOTE before d24cb1a4 gave it a registry row.

just affected upstream/main --no-box selects 140 of 156 gate recipes, since every gate reads crates/placement (a static walk, no dep-info). The change moves two fields and keeps every plan's numbers. The placement and model tests above hold every plan's literal machine, and the next PR's Spark runs (its "What I ran") compare bloomery-serve --plan against main byte for byte on top of this one.

Hardware (if the change is for a machine)

Not for a machine. Mac (Apple M5, 32 GB) and one DGX Spark (GB10, driver 595.84, Ubuntu 24.04, kernel 6.17.0-1032-nvidia) for the aarch64 T1; rustc 1.100.0-nightly (e457a7b0d 2026-08-27), the toolchain rust-toolchain.toml pins.

AI help

AI 코딩 에이전트가 코드를 작성한 후 설명을 썼고, 테스트를 마쳤습니다. diff 리뷰와 PR 초안 리뷰는 제가 직접 했습니다.

@midagedev

Copy link
Copy Markdown
Owner

Thank you for the three Spark PRs (#94, #95, #96). The 0.3.2 release freezes at 13:00 KST today, so these go into review after its tag, in order: #94 first, then #95 on top of it, then #96. They change device and placement code, so each one needs a box run before it lands.

The review bot fails on fork PRs for now; a workflow setting refuses the checkout. That is ours to fix.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Verdict: CLEAN: The field move preserves existing single-node planning; the remaining findings are non-blocking.

Non-blocking

⚠ Test first-node selection: crates/placement/src/placement.rs:261. Every migrated fixture has one node, so replacing first() and first_mut() with last() and last_mut() passes those assertions. Add a fixture with distinct host budgets and pools; check all four accessors and that mutations leave the second node unchanged.

⚠ Attach move-class proof: crates/placement/src/placement.rs:253. The body supplies host/static results but no identical just ptx-scan pair. Attach the covering binaries' base/head scans required by AGENTS.md's “move, split, rename” class; --no-jit suffices.

Checked

Duplicates: the new Node type and 6 methods checked; first-node selection has one owner.
Complexity: the 6 new methods are straight-line accessors and add no model-specific branch.
Silent failure: 3 added-line sites followed. Two expect calls give a named empty-node panic; the moved test fallback selects the defined discrete-memory fixture.
Tests: fixture/accessor edits preserve assertions. Static mutation review: dropping the unified-pool check at crates/placement/src/placement.rs:3627 fails the one-byte-short case; dropping the checkpoint reserve push fails the reserve assertions. First-to-last selection survives the migrated fixtures.
Gates: no pin, threshold, assertion or case removed or relaxed.
Performance: changed callers perform planning or load-time validation; no new work per token, launch, copy or float reduction.
Escape catalog: C1–C4 match no changed code. No ledger lessons are in force.

Notes

This review used source inspection and mutation reasoning; no builds or gates were executed. The supplied evidence lacks a card gate run meeting CONTRIBUTING.md's evidence list.

Card check: needs-gpu. This pull request changes crates/gpu-gates/src/bin/gate_deepseek41_tier.rs, crates/gpu-gates/src/bin/gate_glm5next_residency.rs, crates/gpu-gates/src/bin/shared/residency_clauses.rs … and carries no gate run on a card (CONTRIBUTING's evidence list); a maintainer adds gpu-verified after one.

🔨 Shingler, bloomery's review bot: approves a clean pull request and requests changes on a must-fix (how it works)

Filed outside this PR: #110

…re read and changed through Machine::host, host_mut, unified and unified_mut

Both name this process's node. Every reader outside the struct's own literals goes
through the four accessors, so moving the two fields into a per-node list
(docs/contrib/dgx-spark.md §8.2) changes the accessors and the literals only. No plan
changes: the accessors return the fields as they are.
…n Machine::nodes, this process's node first

Node holds what a node of a machine has beside its cards: its host and, on a
unified machine, the pool its cards and host share. Machine::nodes lists them with this
process's node at 0, which Machine::host, host_mut, unified and unified_mut read; a
machine with no node panics by name there. Every machine built today has one node, so
every plan is unchanged.
@juliankang4

Copy link
Copy Markdown
Collaborator Author

@midagedev 리베이스 완료했습니다. 리뷰 부탁드립니다.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Verdict: CLEAN: No must-fix finding remains. The first-node test gap and missing move-class proof remain non-blocking.

Non-blocking

⚠ Pin first-node selection: crates/placement/src/placement.rs:261 and crates/placement/src/placement.rs:268.
Every changed machine fixture has one node. Replacing first() with last() and first_mut() with last_mut() leaves their assertions green.
Add a two-node case with distinct hosts and pools. Exercise all four accessors and assert that writes leave the second node unchanged. Also assert the named empty-node panic. This pins the contract before per-node planning depends on it.

⚠ Attach the move-class proof: crates/placement/src/placement.rs:253.
The body reports host checks and tests but carries no base/head PTX comparison. AGENTS.md's move class requires identical just ptx-scan results and unchanged host dispatch structural lines.
Attach the affected bins' base/head scans (--no-jit suffices) and the structural checks. Recording this now keeps the proof separate from the next planning change.

Checked

Duplicates: the new Node and six accessor helpers centralize current-node access. Dryer reported no qualifying pairs.
Complexity: crapper flagged no changed functions.
Silent failure: all four accessors reach a named panic on an empty node list.
Tests: all 35 node-vector literals in changed files contain one node; first-to-last selection is equivalent on those fixtures. The diff adds no tests. Returning None from unified() fails the existing integrated-card and pool-bound assertions by inspection.
Gates: no pins, bands, thresholds or cases changed or removed.
Escape catalog: no C1–C4 patterns matched. No ledger lessons are in force.

Notes

Dryer excluded gpu-gates. Validation used source inspection; no builds, tests or hardware gates were executed. The supplied runs do not meet the GPU evidence list.

Card check: needs-gpu. This pull request changes crates/gpu-gates/src/bin/gate_deepseek41_tier.rs, crates/gpu-gates/src/bin/gate_glm5next_residency.rs, crates/gpu-gates/src/bin/shared/residency_clauses.rs … and carries no gate run on a card (CONTRIBUTING's evidence list); a maintainer adds gpu-verified after one.

🔨 Shingler, bloomery's review bot: approves a clean pull request and requests changes on a must-fix (how it works)

@midagedev
midagedev merged commit 159bced into midagedev:main Oct 13, 2026
7 checks passed
midagedev added a commit to juliankang4/bloomery that referenced this pull request Oct 13, 2026
…the machine's node, after midagedev#94 moved host and unified onto nodes[0]
juliankang4 pushed a commit to juliankang4/bloomery that referenced this pull request Oct 14, 2026
…the machine's node, after midagedev#94 moved host and unified onto nodes[0]
juliankang4 pushed a commit that referenced this pull request Oct 14, 2026
…the machine's node, after #94 moved host and unified onto nodes[0]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gpu The card engine: crates/gpu*, crates/app model The CPU engine and model files: crates/model, models, hf, gguf, tokenizer, jinja needs-gpu Changes code a card must check, and carries no gate run on a card placement Placement planning: crates/placement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants