Skip to content

(#5) test: per-hook numeric-difference study - #60

Closed
SieDeta wants to merge 5 commits into
shared-configureable-E2E-libfrom
perhook-numeric-difference
Closed

(#5) test: per-hook numeric-difference study#60
SieDeta wants to merge 5 commits into
shared-configureable-E2E-libfrom
perhook-numeric-difference

Conversation

@SieDeta

@SieDeta SieDeta commented Jun 11, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Summary

This PR implements the numeric equivalence study framework, leveraging the shared comparison infrastructure introduced in previous PRs.

Merge PR #58 first

Key Changes

  • Shared verdict layer

    • Reuses tests.lib.compare from **PR (#3) test: shared E2E lib + configurable matrix #58 ** as the single source of truth for numerical comparisons.

    • Implements the standards-by-mode policy:

      • Eager mode: requires bitwise equality
      • CUDA Graph mode: uses allclose(atol=threshold)

    • Each hook comparison record embeds the shared Check structure, including:

      • Maximum absolute difference
      • Mean absolute difference
      • First differing position
      • Detailed comparison metadata

    This ensures that both the numeric study and the test matrix report drift consistently.

  • Integration with PR2's hardened hook isolation

    • Fully adopts PR2's isolated_hook mechanism.
    • Adds a new --variant compare mode that executes the _compare model inside isolated_hook(...), guaranteeing byte-identical restoration after execution.
    • In this mode, only the selected hook H triggers .copy_().
    • The default --variant p path continues using the production _p + hook_selection=H workflow, which already provides self-isolation.

Deliverables

tests/numeric_study.py

Introduces a CLI utility and run_study() implementation that:

  • Captures an unhooked baseline once.

  • Executes one monitored rollout per hook using:

    • Clean subprocess execution
    • Full [N, vocab] logits capture

For each hook, records:

  • Bitwise equality / allclose verdict
  • Maximum absolute difference
  • Mean absolute difference
  • First differing token position
  • Top-k vocabulary differences at that position
  • Greedy token ID divergence

Outputs:

  • JSON report
  • Human-readable summary table

Failure conditions (non-zero exit status):

  • Numerical alert triggered
  • Token divergence detected
  • Runner error
  • Shape mismatch
  • Empty capture

Alert policy:

  • Eager mode: any non-bitwise difference
  • CUDA Graph mode: differences exceeding the configured threshold
  • All modes: token divergence and runtime failures are always treated as errors

tests/test_numeric_study.py

Adds:

  • 12 CPU unit tests
  • 1 GPU smoke test marked with numeric, gpu, and slow

Verification

Unit Tests

pytest tests/test_numeric_study.py -m cpu -q

Result:

12 passed

Full CPU Test Suite

  • Ran the complete CPU suite excluding the 3 pre-existing broken modules.
  • Result: 122 tests passed, including all tests introduced in this PR.
  • CLI --help output and test collection both complete cleanly.

@SieDeta @claude
test: marker taxonomy + skip guards + categorize (PR1)
4147235 
Introduce the test-suite categorization foundation from the test-suite
refactor plan (PR #1 of 6).

- pyproject.toml: register the full marker set (cpu, gpu, multi_gpu, e2e,
  clickhouse, vllm, hf, ring_native, slow, manual, numeric); default
  collection excludes `manual` and skips tests/tools, tests/ring, .venv,
  integration, build via norecursedirs.
- tests/_requirements.py: CPU-importable skip guards that fail closed with
  precise reasons (require_cuda, require_gpus, require_clickhouse,
  require_vllm, require_model_cache, require_nvcc).
- Mark every test: pure-CPU contract tests -> `cpu`; GPU/E2E suites ->
  explicit gpu/vllm/clickhouse/e2e/hf marks. test_gpt2_parity pulls real
  gpt2 weights -> `hf` + require_model_cache, not cpu.
- Split test_producer_chunked_schema: op-registration tests stay `cpu`,
  CUDA device-smoke tests become `gpu`.
- test_per_hook_isolation: source-patch unit classes -> `cpu`; the slow
  GPU sweep -> `gpu` (keeps `slow`).
- Relocate manual shell wrappers to tests/tools/ (+ README); fix their
  repo-root path computations and cross-references for the new depth.

Default CPU command:
  python -m pytest -m "not gpu and not e2e and not manual" -q

Verified: markers register without warnings; the CPU selection deselects
32 GPU/E2E tests. Pre-existing (reproduced on base main, unrelated to this
PR): monitoring/ring_transport.py references HOOK_TYPE_ROUTER_LOGITS
(defined only in csrc enum, missing on the Python side), and the vendored
transformers submodule is unbuilt -- these block a fully green CPU run and
must be fixed separately.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@SieDeta
SieDeta force-pushed the shared-configureable-E2E-lib branch from 5ceb2be to 017720d Compare June 11, 2026 16:22
@SieDeta
SieDeta force-pushed the perhook-numeric-difference branch from c0ac28a to 15dc826 Compare June 11, 2026 17:08
SieDeta and others added 4 commits June 12, 2026 00:29
@SieDeta
test: marker taxonomy + skip guards + categorize
1a6c400
@SieDeta @claude
test: normalize env vars + harden source-patch
0fd4692 
Phase 1 of the test-suite refactor (plan §2, §6 / PR2).

Env-var normalization — one public hook-selection input:
- E2E_HOOK_SELECTION is now the single public test input; it is
  translated to the internal DMX_HOOK_SELECTION runtime contract (read by
  the runners/adapters) in every subprocess env.
- test_vllm_identical.py: read E2E_HOOK_SELECTION (was E2E_HOOKS); drop
  the duplicate docstring entry.
- test_vllm_rowcnt.py: actually honor the documented E2E_HOOK_SELECTION by
  translating it into DMX_HOOK_SELECTION for the runner/comparator subprocs.
- tools/verify_vllm.sh, tools/identical_vllm.sh,
  tools/run_qwen2_moe_vllm_pipeline.sh: E2E_HOOKS -> E2E_HOOK_SELECTION.
- All E2E_HOOKS usages removed.

Source-patch hardening — single isolation contract:
- Add isolated_hook(framework, model_key, hook) in tests/isolate_hook.py:
  snapshots the vendored _compare source bytes, patches to capture one
  hook, and on exit restores AND asserts byte-identical restoration via a
  SHA-256 compare, raising loudly so a dirty vendored submodule can never
  escape. Invalidates cached bytecode by default so subprocess imports see
  the patched source.
- patch_compare_model kept as a backward-compatible delegating alias.
- Migrate test_per_hook_isolation.py to isolated_hook; add
  test_dirty_restore_raises_loudly covering the loud-failure path.

Acceptance: `pytest -m "not gpu and not e2e and not manual"` — the
per-hook isolation unit suite is green (15 passed, incl. the new restore
test). Pre-existing failures in this checkout stem from the unbuilt
monitoring native layer (HOOK_TYPE_ROUTER_LOGITS) and are unrelated.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@SieDeta @claude
test: shared E2E lib + configurable matrix
6c8f3a6 
Phases 2-3 of the test-suite refactor (plan §7, §8 / PR3). Additive only:
the matrix runs alongside the existing tests and nothing is deleted.

tests/lib/ — shared E2E library (§7), consolidating logic that
vllm_identical_comparator / compare_disk_vs_ch / vllm_rowcnt_comparator /
hf_comparator each reimplemented:
- compare.py: Check dataclass + the four standards (bitwise, allclose,
  row_count, transport_bitwise) as one interface; records max/mean abs +
  first-diff position even on a pass so "barely passing" stays visible.
- align.py: left-pad strip, EOS trim, request_id "<gid>:<row>" parsing,
  vLLM UUID-suffix normalization, logits head-skip alignment.
- report.py: CellResult dataclass -> JSON record / JSONL artifact + human
  table; legacy comparator result.json adapter.
- clickhouse_io.py: canonical dtype map + short-hook->CH act_name map + row
  decode / per-hook counts / request grouping (clickhouse_driver lazy).
- segments.py: thin re-export of monitoring.segment_merger + a sorted
  per-request chunk merge helper.
- disk_ref.py: parse RefDiskWorker .pt filenames; load HF structured refs.
- hf_reference.py: stable-named re-export of the ROL/GEN HF rollouts (the
  700-line module relocation is deferred to the legacy-removal PR).

tests/e2e_matrix.py — configurable matrix entry point (§8) replacing the
hardcoded shell sweeps:
- Multi-value axes (backend/model/mode/standard/hooks/tp/ring/dtype/
  prompt-set) -> Cartesian product of cells.
- Dispatches each cell to the existing runners + comparators as
  subprocesses (no inference logic reimplemented): vLLM bitwise/transport
  -> ref+monitored+vllm_identical_comparator; vLLM row_count/allclose ->
  monitored+vllm_rowcnt_comparator; HF -> ref+monitored+hf_comparator.
- Translates the public E2E_HOOK_SELECTION input to internal
  DMX_HOOK_SELECTION per subprocess (plan §2).
- One JSONL record per cell; per-cell isolation so one bad cell can't abort
  the matrix; per-subprocess --cell-timeout so a hung runner fails the cell.
- --dry-run prints planned cells + dispatch commands with no CUDA/CH, so
  expansion and env translation are CPU-testable.

tests/test_e2e_lib.py — 37 cpu-marked unit tests covering the four
standards, align helpers, report serialization/round-trip, and matrix cell
expansion / env translation / dispatch planning / dry-run.

Acceptance: `pytest -m "not gpu and not e2e and not manual"` — new suite
green (37 passed; 108 total passing on this checkout). Pre-existing
failures stem from the unbuilt monitoring native layer
(HOOK_TYPE_ROUTER_LOGITS) and are unrelated.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@SieDeta @claude
test: per-hook numeric-difference study
4644e19 
Ship the per-hook numeric-difference study (plan §9 / Phase 5): enable one
hook at a time and report the drift its monitoring path introduces versus the
unhooked baseline. Non-goal preserved -- make drift visible/categorized/
reproducible, not fixed. Builds on the PR3 shared lib and the PR2 hardened
isolation contract.

- tests/numeric_study.py: configurable CLI + run_study() orchestration.
  Captures the unhooked baseline once, then one monitored rollout per hook
  (clean subprocess each, full [N, vocab] logits) and records per hook the
  shared-lib Check verdict, max/mean abs diff, first differing token position,
  top-k vocab diffs at that position, and greedy token-id divergence. Emits a
  JSON artifact + human table; non-zero exit on any alert.
    * Reuses tests.lib.compare standards as the §8 standards-by-mode policy:
      eager -> bitwise, cuda_graph -> allclose(atol=per-model/dtype threshold).
    * --variant p (default): production _p Hooked + hook_selection=H (the
      hook_selection mechanism isolates a single hook).
    * --variant compare: _compare variant run inside the PR2 hardened
      tests.isolate_hook.isolated_hook context manager (byte-identical
      restore), so only hook H's .copy_() fires.
- Alert policy: eager -> any non-bitwise diff; cuda_graph -> max abs over
  threshold; always on greedy token divergence, runner error, shape mismatch
  (possible hook identity swap), or empty capture.
- The comparison/alert/report core is pure and CPU-testable; torch is imported
  lazily so the module loads on a torch-less box.
- tests/test_numeric_study.py: 12 CPU unit tests for the verdict logic +
  one numeric/gpu/slow GPU smoke asserting serializable per-hook records and a
  self-consistent eager verdict.

Acceptance (CPU): python -m pytest tests/test_numeric_study.py -m cpu -q

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@SieDeta
SieDeta force-pushed the shared-configureable-E2E-lib branch from 017720d to 6c8f3a6 Compare June 11, 2026 17:39
@SieDeta
SieDeta force-pushed the perhook-numeric-difference branch from 15dc826 to 4644e19 Compare June 11, 2026 17:39
This was referenced Jun 12, 2026
Merged
Open
@Samfisheryu
Samfisheryu force-pushed the shared-configureable-E2E-lib branch from 6c8f3a6 to 2d75054 Compare June 15, 2026 17:33
@Samfisheryu

Samfisheryu commented Jun 15, 2026

Copy link
Copy Markdown
Collaborator

Closing this as superseded by #58. The numeric-study commit from this PR has been cherry-picked into the combined #58 branch for review together with the shared matrix work.

@Samfisheryu Samfisheryu mentioned this pull request Jun 15, 2026
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@SieDeta @Samfisheryu