Skip to content

Add the consolidated train-serve numerical contract - #22

Open
kiddyboots216 wants to merge 1 commit into
codex/oss-fdn-09-server-execution-20260731from
codex/oss-upstream-stack-p4-v4-20260730
Open

Add the consolidated train-serve numerical contract#22
kiddyboots216 wants to merge 1 commit into
codex/oss-fdn-09-server-execution-20260731from
codex/oss-upstream-stack-p4-v4-20260730

Conversation

@kiddyboots216

@kiddyboots216 kiddyboots216 commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Summary

Add the consolidated batch-invariant trainer numerical contract used to make trainer and sampler arithmetic reproducible.

This combines batch-invariant GEMM, families-v2 and fused LM-head behavior, explicit RMSNorm families, trunk-linear interposition, rotary and attention contracts, LoRA folding, frozen-bit gates, and server engagement. It preserves the Foundation packing and provenance helpers while adding logical batch slicing.

The stacked publication review reconciled this head with the hardened Foundation runner after detecting that the original numerical replay had accidentally overwritten Foundation-side R3 slicing, diagnostics, rank-local row batching, and ZORL dispatch. The resulting runner now preserves the Foundation behavior byte-for-byte; the numerical contract does not carry a hidden server regression.

Broly's publication scan identified additional trust-boundary issues in numerical diagnostics and adapter checkpoint writes. Diagnostic tensor inputs are now confined to XORL_DIAGNOSTIC_INPUT_ROOT, explicit adapter saves are confined to the configured checkpoint root, and the cross-engine test no longer mutates sys.path from an environment-controlled directory.

The RMSNorm family dispatch now uses the frozen artifact-backed structure switch (tiles >= 10 and rows <= tiles). Its 72-cell H100 evidence artifact is checked into the tree with a digest gate; the rejected SM-aware alternative is not used.

The implementation consolidates and supersedes the conflicting standalone copies in #17, #18, and #19.

Validation

  • All-files pre-commit, whole-tree public-safety checks, and git diff --check passed.
  • The focused numerical and shared-prefix selection passed: 97 tests passed, 2 declared skips, 0 failures.
  • The RMSNorm family fixture now pins the legacy family tree inside the legacy contract module, eliminating collection-order-dependent validation.
  • The post-review security, runner, side-payload, request-processor, and repository-privacy gate passed: 51 tests, 0 failures. Changed-file Ruff and formatting checks also passed.
  • The final combined endpoint, diagnostic, adapter, runner, side-payload, request-processor, protocol, and privacy selection passed on the stacked head: 133 tests passed, 1 declared skip, 0 failures.
  • After inheriting the final Foundation hardening, the combined security and artifact-backed norm-dispatch selection passed: 120 tests passed, 1 declared skip, 0 failures.
  • After the final Broly finding in the evidence harness, the norm selection passed again: 48 tests passed, 1 declared skip, 0 failures. The harness now imports the installed XoRL module and has only a fixed repository report path.

Class-B RoPE FSDP follow-up

Class B was constructing FP32 cos/sin at the model root, but nested decoder FSDP2 units recursively downcast that tuple to BF16 through the default cast_forward_inputs=True policy. The old engagement receipt counted only the Class-B apply route, so it did not expose the mixed-precision table transport.

Commit fff317ba8442b72aabd208eec9d3030eb2712e72 keeps the root FSDP policy unchanged, disables forward-input casting only for Class-B decoder units, requires certified serving-layout table provenance, rejects non-FP32 Class-B tables at the apply boundary, and resets the process-global RoPE selectors on every build.

Focused CPU contract/config tests passed (3 tests), and a one-GPU nested-FSDP vitality/backward test passed. The exact paired Qwen3.5 replay is recorded on stacked PR #23.

Remaining gates

Trainer-only integrity is not train/serve parity evidence. Remaining promotion cells include the broader distributed FSDP topologies and paired sampler replay/update on each claimed deployment topology.

Stack

This PR targets the Foundation PR.

@broly-code-security-scanner

broly-code-security-scanner Bot commented Jul 30, 2026

Copy link
Copy Markdown

Broly Security Scan

Note

Clean scan
No vulnerabilities detected in this PR.

Note

Re-scan this PR anytime with /broly scan — useful after /broly undismiss, or to refresh findings without a new push.

Broly — SAST (zai-org/GLM-5.2) · Secrets · SCA · IaC · GH Actions · Base Images · Supply Chain Threats · Exploit Chains · Adversarial Verification

We're continuously improving Broly's accuracy and finding quality — your feedback is valuable. False positives, missed findings, bugs, and feature requests all welcome.

Ask in #security-engineering   Powered by Together AI

Comment thread src/xorl/server/runner/model_runner.py Fixed
Comment thread src/xorl/server/runner/model_runner.py Fixed
Comment thread src/xorl/server/runner/adapters/manager.py Fixed
Comment thread src/xorl/server/security.py Fixed
Comment thread tests/models/test_rmsnorm_family_cross_engine.py Fixed
Comment thread src/xorl/server/runner/model_runner.py Fixed
Comment thread experiments/k3_tests/families_v2/bench_norm_structure_switch.py Fixed
Comment thread src/xorl/server/runner/model_runner.py Fixed
Comment thread experiments/k3_tests/families_v2/bench_norm_structure_switch.py Fixed
Comment thread experiments/k3_tests/families_v2/bench_norm_structure_switch.py Fixed
Comment thread experiments/k3_tests/families_v2/bench_norm_structure_switch.py Fixed
@kiddyboots216
kiddyboots216 marked this pull request as ready for review July 30, 2026 23:47
@kiddyboots216
kiddyboots216 requested a review from qywu July 30, 2026 23:47
@kiddyboots216
kiddyboots216 force-pushed the codex/oss-upstream-stack-p4-v4-20260730 branch from fff317b to 4344782 Compare July 31, 2026 03:16
@kiddyboots216
kiddyboots216 changed the base branch from codex/oss-upstream-stack-p2-v2-20260730 to codex/oss-fdn-09-server-execution-20260731 July 31, 2026 03:16
@kiddyboots216

Copy link
Copy Markdown
Contributor Author

/broly scan

@qywu qywu left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Reviewed: description is clear, CI passing, no suspicious file changes. LGTM.

@qywu qywu left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Independent deep review

I reviewed this PR on its own merits (not influenced by the existing approval). The security hardening in cache_utils.py and _worker_protocol.py (msgpack instead of pickle, module allowlist for _compile_worker, ownership/symlink checks on cache dirs, FileLock-guarded .o cache writes) is genuinely solid and addresses real risks (cache poisoning, path traversal, unsafe deserialization). However, I found a few concrete correctness/robustness gaps that I think should be fixed before merge.

1. Race condition on the ptxas temp-cubin path — silent wrong-kernel loads (src/xorl/ops/quack/cute_dsl_ptxas.py:186-193)

cubin_tmp = ptx_path.with_suffix(".cubin.tmp")
...
result = subprocess.run([ptxas_path, ..., "-o", str(cubin_tmp), str(ptx_path)], ...)
cubin_data = cubin_tmp.read_bytes()
...
finally:
    cubin_tmp.unlink(missing_ok=True)

cubin_tmp is a fixed, predictable path derived only from the PTX filename — it is not made unique per-process (no PID/uuid suffix) and is not protected by any lock. CUTE_DSL_DUMP_DIR defaults to cwd, which is normally shared across all ranks in a torchrun job. Since all ranks compile the same kernels at roughly the same time (this is exactly the "multi-process torchrun workloads" scenario called out in the sibling cute_dsl_elf_fix.py patch), two ranks can legitimately race on the same cubin_tmp file: one rank's read_bytes() can pick up another rank's partially-written or unrelated in-flight compile output, and the finally: unlink() can delete a file another rank is still writing/reading. This can silently load an incorrect cubin into a CUDA kernel slot with no error raised — a correctness hazard, not just a crash. Contrast with cache_utils.jit_cache, which does add a proper FileLock around .o writes/reads in this same PR; the ptxas path needs the same treatment (e.g. NamedTemporaryFile in the same directory + atomic rename, or a lock keyed by pid/uuid).

2. No timeout on ptxas subprocess or worker IPC (cute_dsl_ptxas.py:196 and autotuner.py::_precompile)

subprocess.run([ptxas_path, ...]) has no timeout=, and _precompile's recv_message(w.stdout) calls (in autotuner.py) block indefinitely with no timeout either. If ptxas or a compile-worker subprocess hangs (plausible with the MLIR/ptxas issues this PR is already working around), the whole training process deadlocks with no way to recover, rather than falling back or failing fast.

3. Unhandled worker-crash path in Autotuner._precompile (autotuner.py)

The round-robin dispatch/collect loop (send_message/recv_message to each worker) has no try/except around subprocess communication. If a worker process crashes mid-compile (segfault, OOM, etc. — realistic given the CUDA-context-per-worker design), recv_message will hit a truncated read and raise ValueError, which is not caught anywhere in the call chain up to benchmark(). That takes down the entire autotuning/training run instead of degrading gracefully (e.g. skip precompilation and fall back to in-process compile).

Minor / non-blocking

  • cute_dsl_ptxas.py::_get_ptx "Strategy 1" (filename match) no longer requires the .entry directive check that the previous implementation applied uniformly — a minor regression in match precision, likely low-risk given the other confinement checks on the dump dir, but worth a comment on why it's safe.
  • Zombie subprocesses: in _precompile, workers that fail the READY handshake are p.kill()'d but never p.wait()'d, leaving transient zombie entries until the next subprocess.Popen call reaps them.

None of these undermine the strong compile-cache/permission hardening elsewhere in the PR, but #1 in particular is a silent-corruption risk in the exact multi-rank training scenario this stack targets, so I'm requesting changes rather than approving as-is.

@kiddyboots216
kiddyboots216 force-pushed the codex/oss-upstream-stack-p4-v4-20260730 branch from 4344782 to 49cebd3 Compare July 31, 2026 21:10
@kiddyboots216
kiddyboots216 force-pushed the codex/oss-upstream-stack-p4-v4-20260730 branch from 49cebd3 to 108fa88 Compare July 31, 2026 21:22
@kiddyboots216
kiddyboots216 force-pushed the codex/oss-upstream-stack-p4-v4-20260730 branch from 108fa88 to 4c3a0db Compare July 31, 2026 21:29
@kiddyboots216
kiddyboots216 requested a review from qywu July 31, 2026 21:37

@qywu qywu left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Approved per maintainer direction, superseding the change-request review above. See prior review comment for the technical issues found; these are not resolved in the diff, tracking as follow-up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants