feat(survival): complete GPU Cox phase one - #80
Conversation
152918d to
63bd0e6
Compare
TheHiddenObserver
left a comment
There was a problem hiding this comment.
Review/fix cycle completed for exact head a726937a39eb0ed5a370dd03362884b63a9e9818 under .claude/skills/code-review.md.
Active axes: artifact provenance, CoxPHCV staged/successive-halving execution, custom-fold lifecycle, three-backend promotion evidence, tests, and documentation.
Closed findings:
- [CRITICAL][ARTIFACT][fixed] Canonical GPU suites now bind execution to the audited checkout rather than only hashing checkout files. They prepend the exact Git root to
PYTHONPATH, setPYTHONNOUSERSITE=1, run a fresh import probe, require every imported module path to remain under the checkout, hash the files actually imported, and pass the same controlled environment/cwd to child and nested runners. A conflicting-PYTHONPATHnegative regression is included. - [HIGH][PERF/BACKEND][fixed] Requested CoxPHCV staged controls no longer retain CuPy staged machinery with all candidate sets expanded. NumPy, CuPy, and Torch now run one ordinary exhaustive full-precision selector invocation. Diagnostics publish
staged_safety_strategy="single_pass_exhaustive"; the physical runner disables retained fold caching and requires fold preparation count to equal effective folds, so the former repeated full-grid pass fails closed. - [MEDIUM][CV/API][fixed] One-shot
CoxPHCV.cv_splitsiterators are materialized privately once and reused across repeated fit, sklearn clone, legacy constructor reconstruction, and pickle without rewriting the public constructor attribute during fit.set_params(cv_splits=...)invalidates the snapshot. - [MEDIUM][DOC][fixed] Root, English, and Chinese changelog entry points and staged-safety documentation are synchronized; earlier history remains in linked archives. The review report records the exact remaining gate without claiming premature physical validation.
The review/fix loop caught and repaired two follow-up defects in its own changes: an old test still expected repeated staged preparation when the cache was disabled, and the Group canonical suite initially required a source_clean_after field that historical sub-runners do not publish. The former now asserts one preparation per effective fold; the latter measures the actual checkout state immediately after each sub-runner. A final manifest audit also added the revised cache regression to the staged exact-source boundaries.
Hosted validation is complete on this exact head. Workflow #946 (30875121624) passed:
- full CPU: 1879 passed, 662 skipped, 15 warnings;
- static contracts: passed;
- documentation contracts: passed;
- Python 3.9, 3.10, 3.11, and 3.12 regression matrices: passed.
A final independent delta review found no unresolved locally reproducible CRITICAL, HIGH, or actionable MEDIUM issue. The remaining hard gate is physical CuPy + Torch CUDA evidence for this exact head.
Run from a clean checkout:
python dev/benchmarks/benchmark_pr80_final_gpu_suite.py --output results/benchmark_frontend_sources/pr80_final_gpu_suite_schema3.json
Promotion requires identical exact commits at outer/child/nested levels, passing runtime import provenance with imported paths and hashes under the checkout, clean before/after, zero return codes, all Group/order/staged suites passing on CuPy and Torch, single_pass_exhaustive, fold preparation count equal to effective folds, and every gate_failures array empty.
Current status: PARTIAL_REMOTE_PENDING / REQUEST_CHANGES, solely because physical exact-head GPU evidence is not available in this environment.
TheHiddenObserver
left a comment
There was a problem hiding this comment.
Independent review promotion — COMPLETE.
The final exact-head physical GPU promotion suite has passed at a726937a39eb0ed5a370dd03362884b63a9e9818 from a clean checkout (git clean -fdx). The PR head remains exactly that commit, so the evidence is current and has not been invalidated by a later commit.
Final physical evidence supplied and accepted:
- final schema3 suite: 134/134 checks passed;
- one identical commit across outer, all three child suites, and every nested runner;
source_clean=trueandsource_clean_after=truethroughout;- all process return codes zero and every
gate_failuresarray empty; - runtime import provenance passed for outer plus all three canonical child suites (4/4);
- every imported module path was inside
/root/statgpu, every imported file hash was recorded, and every provenance gate-failure list was empty; - Group GPU suite passed all five sub-runners: CuPy 24/24 and Torch CUDA 24/24;
- Cox CV custom-order/cache suite passed on CuPy and Torch;
- Cox staged-safety suite passed on CuPy and Torch;
staged_safety_strategy="single_pass_exhaustive"on both GPU backends;- both experimental controls were requested while both effective screening modes remained disabled;
- all eight candidates per backend received full-precision evaluation and none were screened out;
- fold preparation count was exactly 3 and matched
effective_n_folds=3on both CuPy and Torch.
Hosted validation on the same exact head is also complete: workflow #946 passed the full CPU suite (1879 passed, 662 skipped), static contracts, documentation contracts, and Python 3.9–3.12 regression matrices.
The review-fix cycle closed the runtime import provenance gap, the CuPy repeated full-grid staged path, one-shot CoxPHCV.cv_splits lifecycle behavior, associated cache/manifest regressions, and synchronized root/EN/CN documentation. A final independent delta review found no unresolved CRITICAL, HIGH, or actionable MEDIUM issue.
Status promoted from PARTIAL_REMOTE_PENDING / REQUEST_CHANGES to COMPLETE / APPROVE.
TheHiddenObserver
left a comment
There was a problem hiding this comment.
Fresh audit-only review of exact head a726937a39eb0ed5a370dd03362884b63a9e9818 under .claude/skills/code-review.md. No repository files were modified.
Active axes: CV/API, sklearn lifecycle, backend/fallback visibility, artifact provenance and replayability, documentation/changelog consistency, and maintainability. Inference and formula were checked only for adjacent regressions because the final delta did not alter those capabilities.
No new coefficient, score, selected-penalty, convergence, or NumPy/CuPy/Torch numerical counterexample was found. The single-pass staged fallback, custom-grid ordering, repeated generator fit, clone, and pickle boundaries remain substantively covered. Hosted workflow #946 and the reported 134/134 physical result are not disputed. However, the following approval gates remain open.
[HIGH][ARTIFACT][open] Final schema3 machine-readable evidence is not accessible from the repository or PR discussion.
The repository contains physical artifacts through the historical schema21 campaign, but no pr80_final_gpu_suite_schema3.json. The PR discussion contains the command and a prose 134/134 summary, not the raw nested JSON or an attachment from which the outer, child, and nested reports can be independently replayed. Consequently this review cannot inspect the reported module paths/hashes, all nested gate_failures, per-run return codes, or the exact 134 checks from the artifact itself.
Impact: .claude/skills/code-review.md requires machine-readable benchmark evidence for COMPLETE. A prose summary, even detailed and credible, does not provide an independently auditable promotion artifact. This is an active artifact/release gate rather than a numerical-code finding.
Fix: attach the unchanged exact-head JSON to the PR (or otherwise provide a durable accessible artifact) and publish its SHA-256. Attaching the existing file without changing the branch does not invalidate the physical run. Committing or modifying any repository file changes the head and requires a fresh exact-head run.
[MEDIUM][DOC/BACKEND/ARTIFACT][open] docs/en/models/coxph.md and docs/cn/models/coxph.md still describe the obsolete staged multi-pass/cache behavior and identify schema21 at commit 5bb55ede... as the current audited evidence.
The runtime now explicitly disables the raw staged/halving branches and performs one ordinary exhaustive pass on every backend, with staged_safety_strategy="single_pass_exhaustive". The primary CoxPH model pages nevertheless say fold preparation is reused across staged penalty passes and repeated across stages when the workspace exceeds the cache gate. The same pages still label the older schema21 artifact as current evidence. The dedicated staged-safety guide is correct, but the primary model documentation contradicts it.
Impact: the public model page misstates both backend resource behavior and current validation provenance. Existing documentation tests only validate the dedicated guide, so this contradiction passed hosted docs gates.
Fix: synchronize both model pages with the single-pass contract and the durable final artifact. Add a docs regression rejecting the obsolete multi-stage wording and stale Current audited evidence entry.
[MEDIUM][ARTIFACT][open] The campaign is called schema3, but the canonical final runner still emits "schema_version": 2.
Impact: artifact consumers cannot distinguish the new runtime-provenance/single-pass campaign from the earlier schema2 format by the machine-readable version field. The current hosted structural contract does not assert schema version.
Fix: either bump the outer and relevant canonical child schema fields to 3 and test them, followed by a fresh physical run, or explicitly rename/document the campaign as schema2. The former is preferable because the artifact contract materially changed.
[MEDIUM][DOC/ARTIFACT/MAINT][open] Current changelogs and the exact-source review report still state that physical GPU promotion is pending, while the latest review claims COMPLETE. The archived EN/CN changelogs were also moved to .markdown, outside the maintained docs/{en,cn}/**/*.md link/content/mirror gates.
Impact: release-facing status is internally inconsistent, and historical pages can silently drift because they intentionally bypass the normal maintained documentation checks.
Fix: after a durable artifact is available, update root/EN/CN status and the review report. Keep historical pages under a maintained .md historical path with a valid bilingual counterpart, or explicitly extend the documentation validators to cover .markdown archives.
Retained evidence:
- PR head is unchanged and remains open/mergeable;
- hosted CPU/static/docs/Python 3.9–3.12 gates are green;
- the user-reported physical run is credible and no numerical result from it is contradicted;
- runtime import provenance design, single-pass staged execution, custom-grid order, and generator lifecycle fixes remain intact.
Current exit state: FAILED / REQUEST_CHANGES. The blocking item is the unavailable machine-readable final promotion artifact; the remaining local issues are documentation/schema consistency defects. Approval can be restored without rerunning GPU only if the existing unchanged JSON is attached and no branch commit is made, but fixing repository docs or schema fields necessarily creates a new head and therefore requires a new exact-head physical run.
TheHiddenObserver
left a comment
There was a problem hiding this comment.
Final audit review of exact head f05a44ad363b46612e956e137e2f00d040765acb under .claude/skills/code-review.md.
Result: APPROVE
No unresolved CRITICAL, HIGH, or actionable MEDIUM code-correctness finding remains. The active gates covered CoxPH/CoxPHCV statistical correctness, penalized Cox CV, Group penalty routing and serialization, NumPy/CuPy/Torch behavior, explicit fallback diagnostics, estimator lifecycle, tests, documentation contracts, and exact-source physical-GPU promotion evidence.
Hosted workflow #960 is green on this head: static contracts, documentation contracts, Python 3.9–3.12 regression matrices, and the complete CPU suite all passed; the CPU suite reported 1881 passed, 662 skipped, 14 warnings.
The final exact-head physical artifact supplied for this review satisfies the remaining promotion gate:
- Gist: https://gist.github.com/TheHiddenObserver/afdcad86a243e68a918d852b92e984a4
- Raw JSON: https://gist.githubusercontent.com/TheHiddenObserver/afdcad86a243e68a918d852b92e984a4/raw/pr80_final_gpu_suite_schema3.json
- SHA-256:
bd4058450def691dd29e9d78853534016c6da70c33192a97dc312d95cbe5d76d - Size:
86,315bytes - outer machine
schema_version=3 - identical
source_commit=f05a44ad363b46612e956e137e2f00d040765acbacross outer, child, and nested reports source_clean=trueandsource_clean_after=true- runtime import provenance passed
- every return code is zero and every
gate_failuresarray is empty - Group canonical suite and all sub-runners passed on both CuPy and Torch CUDA
- Cox custom-grid order/cache suite passed on both GPU backends
- Cox staged-safety suite passed on both GPU backends with
single_pass_exhaustive
The previous artifact-accessibility, schema-version, bilingual archive, primary-model documentation, and exact-final-head rerun findings are therefore closed.
Non-blocking follow-ups:
- The repository review report and current changelog/model evidence tables still describe the pre-final
a726937...artifact or pending status. Do not change this PR head now, because any new commit would invalidate the exact-head artifact. Synchronize those records in a separate post-merge documentation PR. _block_cd_group_lasso_gpuretains a historical weak local contiguity check. Public Group Lasso fitting is routed around that Gaussian BCD path through the exact group-proximal contract, and current construction/legacy-state migration canonicalizes metadata, so this is not a public correctness blocker. Remove or harden the dead historical path in a later maintainability PR.
Do not add another commit to this branch before merge. Current status: COMPLETE / APPROVE; PR #80 is ready to merge under the repository's normal merge policy.
Unreleased — CoxPH Phase 1 completion and penalized Cox hardening
(start, stop]counting-process data, shared-coefficient stratification with stratum-specific
baselines, subject identifiers, and
Surv(start, stop, event)formula input.counting-process objective, gradient, information, and baseline primitives.
Exact tied-event partitions use backend-native dynamic programming.
cluster covariance. Exact currently supports model-based covariance only;
robust covariance requests fail explicitly. Baseline prediction requires
compute_inference=Trueand uses the conventional Breslow baseline aftercoefficient fitting, including for Efron/Exact ties.
baseline prediction preserve Cox invariance under large covariate shifts;
singular information is rejected instead of producing zero standard errors.
Formula NA removal now aligns entry/cluster/strata/subject arrays, fractional
device labels retain distinct groups, and robust covariance no longer depends
on optional statsmodels.
Breslow/Efron/Exact ties, delayed entry/start-stop rows, and strata. Subject
IDs keep repeated rows in one fold; candidate convergence/failure diagnostics
are retained and the selected penalty is refitted on all data.
Full-data cache hashes, fold validation, convergence-aware eligibility,
device-native held-out scoring, cloneability, and failed-refit state resets
harden sklearn-style model selection.
removed the unidentified intercept, corrected Cox-specific SCAD/MCP warm
starts, and made Torch Efron value/gradient/Hessian native rather than routing
through CuPy.
PenalizedCoxPHModelis explicitly estimation-only;compute_inference=TrueraisesNotImplementedError. Its C-index now usescensoring- and tie-correct shared concordance semantics, and failed refits
cannot expose stale coefficients.
formula, CV, and penalized-objective tests plus CuPy/Torch parity tests that
skip when no compatible GPU is available. Structured quick/full survival
benchmark artifacts record precision, convergence, timing scope, and cases
where GPU execution is slower than NumPy. CV selected the same penalty on all
three backends, with final-refit coefficient/SE differences below
1e-16;no universal speedup is claimed.