Skip to content

fix: harden runtime and estimator contracts - #87

Merged
TheHiddenObserver merged 394 commits into
masterfrom
agent/maintenance-0.2.4-0.2.5
Aug 6, 2026
Merged

fix: harden runtime and estimator contracts#87
TheHiddenObserver merged 394 commits into
masterfrom
agent/maintenance-0.2.4-0.2.5

Conversation

@TheHiddenObserver

@TheHiddenObserver TheHiddenObserver commented Aug 4, 2026

Copy link
Copy Markdown
Owner

Summary

This maintenance PR addresses Issues #45, #81, #82, and #83 without changing the package version or publishing a release. It completed repeated review → fix → targeted validation → independent re-review cycles under .claude/skills/code-review.md.

Current status: REVIEW_COMPLETE_MERGE_READY_SQUASH_ONLY.

  • current head: 9397171c29b3a98f21f5d475f54a433d44ad8be2;
  • no unresolved CRITICAL, HIGH, or in-scope MEDIUM findings remain;
  • all current-head hosted gates pass;
  • the physical Torch/CuPy numerical gate passed on 1c44fe7d1047c91b1a7abd5a65a48140fd81ef54;
  • subsequent changes are limited to the backend-agnostic import-cycle repair, its fresh-interpreter regression, and root/English/Chinese changelog entries;
  • this PR must be squash-merged only.

Review-fix closure

The review cycles closed correctness, backend, inference, API, fallback, CV, formula, solver, estimator-state, and documentation gaps, including:

  • correct arbitrary-link Binomial IRLS Fisher weights, working response, line-search objective, warm-start normalization, and quadratic-penalty validation;
  • strict LogisticRegression response/control validation, transactional fit state, visible convergence, backend-native analytic weights, integer hard predictions, stable likelihood diagnostics, and one-class metric behavior;
  • failure-safe RidgeCV, ElasticNetCV, LogisticRegressionCV, and unified penalized-CV scoring/refit behavior with explicit backend pinning and narrow fallbacks;
  • finite-input validation, formula-row alignment, sklearn cloning/tags, nested set_params, and inference publication contracts;
  • correct smooth/non-smooth solver dispatch, observable Torch compile fallback behavior, and narrowed numerical recovery;
  • removal of the statgpu.glm_core / Cox-loss package-initialization cycle by lazily exporting CoxPartialLikelihoodLoss, so fresh-interpreter imports no longer depend on importing LogisticRegression first.

Physical GPU evidence

Validated on source head 1c44fe7d1047c91b1a7abd5a65a48140fd81ef54.

Torch compile and CUDA Graph

Environment: RTX 4090, PyTorch 2.8.0+cu128.

  • selected compile/CUDA Graph matrix: 9 passed in 9.52s;
  • IRLS coefficients: [0.0, 1.0];
  • prediction dtype: torch.int64;
  • weighted logistic log-likelihood: -0.557463468137747;
  • all runtime assertions passed.

CuPy runtime contracts

Environment: Tesla P100-SXM2-16GB, CuPy 13.6.0.

  • IRLS coefficients: [0.0, 1.0];
  • prediction dtype: int64;
  • weighted logistic log-likelihood: -0.5574634681377472;
  • all runtime assertions passed.

No backend kernel, solver, fitting, prediction, inference, or numerical implementation changed after this physical GPU gate. The later source repair affects package initialization only and has a fresh-interpreter regression test.

Documentation

The PR updates the root, English, and Chinese changelogs; LogisticRegression and ElasticNet model guides; cross-validation guides; solver algorithm and solver/penalty matrix guides; developer test ownership guidance; and manual GPU diagnostic policy.

The final import-cycle repair is explicitly recorded in:

  • CHANGELOG.md;
  • docs/en/changelog.md;
  • docs/cn/changelog.md.

Current-head hosted validation

Validated on 9397171c29b3a98f21f5d475f54a433d44ad8be2:

  • Tests #1399: success
    • complete CPU suite: 2239 passed, 719 skipped, 20 warnings in 36.67s;
    • fresh-interpreter import-order regression: pass;
    • static and documentation contracts: success;
    • Python 3.9, 3.10, 3.11, and 3.12 regression jobs: success;
  • Maintenance compatibility #412: success;
  • Release notes validation #426: success;
  • Release package validation #432: success, including clean sdist installation and Linux/macOS/Windows wheel smoke tests.

Torch compile policy

Internal Torch kernels remain eager when STATGPU_TORCH_COMPILE_MODE is unset, auto, or disable. default and reduce-overhead remain explicit opt-ins. Compile construction/runtime fallback remains observable, and only the known CUDA Graph overwritten-output lifecycle failure becomes a permanent eager fallback.

Merge hygiene

The final tree contains no temporary review workflow, patch script, or CI trigger file. The branch history contains temporary review/validation commits, so merge by squash only. Do not use a merge commit or history-preserving rebase merge.

Issue status

@TheHiddenObserver
TheHiddenObserver marked this pull request as ready for review August 4, 2026 13:31
Copilot AI lite review requested due to automatic review settings August 4, 2026 13:31
@cursor

cursor Bot commented Aug 4, 2026

Copy link
Copy Markdown

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@TheHiddenObserver TheHiddenObserver left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Code review verdict: REQUEST CHANGES

Audit mode under .claude/skills/code-review.md.

Active axes: backend, solver/penalty runtime dispatch, public estimator API, sklearn/CV integration, formula/input boundaries, tests/workflows, and performance. Statistical objective and inference formulas are not changed directly.

Findings

[CRITICAL][FALLBACK][open] statgpu/backends/_torch_compile.py:86-101 — compile construction failures silently return the eager callable.

Impact: this is an unobservable compiled→eager fallback. The physical GPU test verifies Torch output correctness but does not prove that torch.compile successfully constructed or executed a compiled graph; the test also passes when construction is rejected and compile_torch() returns fn. This violates the repository rule that fallback must be visible and that tests must not allow silent fallback.

Fix: expose a structured compile state/reason on every returned callable, warn or report construction failure, and make the physical test assert that the relevant Lasso kernel actually entered mode="default" compiled execution before checking repeated-fit correctness.

[HIGH][API][open] statgpu/_base.py:740-780set_params() does not refresh normalized/validated runtime state.

Impact: _statgpu_refresh_normalized_params is called but no estimator implements it. Valid constructor inputs can become invalid runtime state through set_params(). For example, PooledOLS(cov_type="HAC") is accepted and normalized by __init__, while PooledOLS().set_params(cov_type="HAC") leaves self.cov_type == "HAC"; _compute_inference() then matches no covariance branch and can reference cov_params before assignment. This fails Issue #82's required design and acceptance criteria.

Fix: preserve exact public attributes and maintain validated private normalized attributes (_cov_type, _solver, etc.), with a real shared refresh/validation path invoked by both __init__ and set_params().

[HIGH][API][open] statgpu/_base.py:640-680 — the new sklearn tags misclassify most public estimators.

Impact: __sklearn_tags__() derives estimator_type only from _estimator_type, but LogisticRegression, Ridge, and most other public estimators do not define it. Under current sklearn they are therefore reported as neither classifier nor regressor. This can select KFold instead of StratifiedKFold for classifiers and breaks is_classifier/is_regressor and metadata routing expectations. The current integration test covers only Ridge with an explicit scorer, so it does not detect this.

Fix: define correct estimator-family tags/mixins and test is_classifier(LogisticRegression()), is_regressor(Ridge()), default classifier GridSearchCV/CV splitter behavior, and transformer tags under legacy and current sklearn.

[HIGH][MATRIX][open] statgpu/_base.py:31-150, statgpu/backends/_validation.py, and public estimators — finite-input coverage is incomplete.

Impact: validation is driven by method-name and parameter-name allowlists. Public numerical paths such as PCA.inverse_transform(X) are not wrapped and only check shape, so NaN/Inf propagate. ElasticNet.fit(initial_coef=...) is wrapped, but initial_coef is not in _FINITE_PARAMETER_NAMES. Numerical object ndarrays also bypass check_finite() because object dtype is traversed only for list/tuple inputs. All pandas objects are deferred, including non-formula numerical calls. Thus Issue #81's “all public numerical entry points” acceptance criterion and the changelog claim are not met.

Fix: generate a public-method/parameter inventory, cover inverse/score/inference/custom prediction methods and initialization aliases, validate numerical object arrays, and document only intentional missing-value support.

[HIGH][MATRIX][needs remote GPU] dev/tests/test_maintenance_024_025.py — runtime matrix coverage is insufficient for the changed compiled kernels and new finite-validation backend contract.

Impact: Issue #45 lists 11 compiled call sites spanning L1, ElasticNet, SCAD, MCP, group, adaptive, and FISTA paths, but exact-head physical GPU evidence covers only one Lasso fit. The new finite validator has a Torch CPU unit test but no exact-head CuPy rejection/device-purity regression. The code-review matrix requires non-smooth, nonconvex, group, adaptive, and three-backend coverage for touched solver/penalty behavior.

Fix: add parameterized modern-CUDA tests for representative ElasticNet, SCAD/MCP, group and adaptive paths (or direct compiled proximal kernels), plus exact-head CuPy/Torch finite-rejection and scalar-only synchronization assertions.

[MEDIUM][PERF][needs optimization] compiled mode changed across performance-sensitive kernels without benchmark evidence.

Impact: replacing reduce-overhead with default is justified for correctness, but warm-up and steady-state performance are unmeasured. The review policy requires benchmark evidence or an optimization note/caveat for performance-sensitive changes.

Fix: record representative eager/default timings and memory behavior on the RTX 4090 for Lasso, ElasticNet, and one nonconvex/group path; document any expected regression.

[MEDIUM][DOC][open] PR/changelog completion claims are broader than the evidence.

Impact: the changelog states that public estimator numerical inputs are uniformly checked, while the allowlist gaps above remain. The PR also records a public server host/port that is unnecessary for reproducibility.

Fix: narrow the claims until the matrix is complete, preserve GPU model/software/commit information, and remove the server endpoint.

Positive evidence

  • exact-head hosted CI is green;
  • full CPU suite and Python 3.9–3.12 matrix passed;
  • wheel/sdist smoke tests passed on Ubuntu, Windows, and macOS;
  • the reported RTX 4090 repeated Lasso fit confirms that the original CUDA Graph lifecycle crash is not reproduced in that tested configuration.

These results are valuable, but they do not close the blocking fallback, sklearn contract, and coverage findings above.

@TheHiddenObserver
TheHiddenObserver marked this pull request as draft August 4, 2026 15:30

@TheHiddenObserver TheHiddenObserver left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Auto-fix follow-up review for exact head f1ef6fcd0c3a0dbbfa0f8f93bb4447e0860caf6f.

[CRITICAL][FALLBACK][fixed] statgpu/backends/_torch_compile.py
Impact: compile construction/runtime fallback is no longer silent.
Fix: bounded diagnostics and callable status attributes were added; construction fallback warns; runtime fallback is restricted to the CUDA Graph tensor-output overwrite signature; unrelated RuntimeError values are re-raised.
Evidence: maintenance regressions and complete hosted suite pass.

[HIGH][API][fixed] statgpu/_base.py
Impact: set_params() now refreshes normalized runtime state without moving intentional fit-time validation into the constructor boundary.
Fix: valid updates use a transactional fresh-constructor rebuild; constructor-rejected values retain the established estimator-specific fit validation boundary; fitted state is reset for rebuilt estimators.
Evidence: PooledOLS normalization, Cox fit-boundary, clone, Pipeline, and GridSearchCV contracts pass.

[HIGH][API][fixed] statgpu/_base.py
Impact: sklearn classifier/regressor metadata is now consistent across sklearn 1.2.2 and current sklearn.
Fix: internal non-predictive families are not misclassified, predictive families expose compatible tags, and external subclasses preserve the parent estimator type.
Evidence: LogisticRegression, Ridge, GraphicalLasso, external Ridge subclass, legacy clone inventory, and current sklearn tests pass.

[HIGH][MATRIX][fixed] statgpu/_base.py, statgpu/backends/_validation.py
Impact: maintained public numerical boundaries now cover inverse-transform, scoring, initialization arrays, panel IDs, sparse data, pandas numerical data, and object arrays while preserving formula-owned missing-row semantics.
Evidence: NumPy/Torch tests and full CPU suite pass; CuPy device test is present for remote execution.

[HIGH][MATRIX][needs remote GPU] dev/tests/test_maintenance_024_025.py
Impact: the updated physical matrix now verifies actual Dynamo graph creation and L1/adaptive/nonconvex/group compiled paths instead of accepting a hidden eager fallback.
Evidence required: exact-head Torch and CuPy commands recorded in the PR body.

[MEDIUM][PERF][deferred]
Impact: no performance equivalence claim is made between default and reduce-overhead compile modes.
Fix: the limitation and optimization follow-up are documented; correctness and observable fallback are the maintenance-release gates.

Hosted evidence: Tests #1046, Maintenance compatibility #59, Release notes #73, and Release package validation #79 all succeeded. Full CPU suite: 1894 passed, 667 skipped. Python 3.9–3.12 and Ubuntu/Windows/macOS wheel smoke all succeeded.

Exit status: PARTIAL_REMOTE_PENDING. No unresolved local CRITICAL/HIGH findings remain; PR remains draft pending the updated exact-head physical GPU matrix.

@TheHiddenObserver TheHiddenObserver left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Final review/fix-loop result

Reviewed the clean exact head 60071b02b0fcb5c51111b52cf3facbbc4a0df48b under .claude/skills/code-review.md after completing the repeated review → fix → re-review cycle.

Latest review result

No new CRITICAL, HIGH, or in-scope MEDIUM findings were identified in the final independent review.

The final review rechecked:

  • centralized Torch compile policy, call-site exception handling, diagnostics, and fallback boundaries;
  • public finite-value guards across inherited, mixin, formula, Cox, panel, transform, score, and knockoff paths;
  • public-raw/private-normalized constructor state;
  • legacy and current sklearn get_params, transactional set_params, tags, recursive nested clone, Pipeline, and GridSearchCV behavior;
  • one-shot CoxPHCV.cv_splits fit/clone/pickle lifecycle;
  • custom get_params() implementations with nondefault mutable/string/NumPy-scalar values;
  • supervised and nonpredictive estimator classifications;
  • removal of all temporary review scripts and write-enabled bootstrap workflows.

Exact-head hosted evidence

  • Tests #1103: success — 1916 passed, 667 skipped; static/docs and Python 3.9–3.12 matrix passed.
  • Maintenance compatibility #116: success under sklearn 1.2.2.
  • Release notes validation #130: success.
  • Release package validation #136: success, including clean sdist and Ubuntu/Windows/macOS wheel smoke tests.

Exit state

REVIEW_CLEAN_REMOTE_GPU_PENDING

The code-review loop is complete. The PR remains Draft only because current-head physical Torch/CuPy evidence has intentionally not yet been run.

@TheHiddenObserver
TheHiddenObserver marked this pull request as ready for review August 5, 2026 02:12
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

Copy link
Copy Markdown
Owner Author

Exact-head physical GPU gate — PASS

Validated against commit 60071b02b0fcb5c51111b52cf3facbbc4a0df48b.

Torch

  • NVIDIA GeForce RTX 4090, compute capability 8.9
  • PyTorch 2.8.0+cu128
  • test_torch_lasso_py21_iterative_compile_smoke: PASS
  • test_physical_cuda_compile_path_is_observable: PASS
  • test_torch_penalty_compile_matrix_py21: PASS
  • Result: 3 passed in 16.04s, exit code 0

CuPy

  • Tesla P100-SXM2-16GB
  • CuPy 13.6.0
  • test_cupy_finite_validation_stays_on_device: PASS
  • Result: 1 passed in 2.26s, exit code 0

Combined result: ALL PASS. Together with the completed review/fix loop and exact-head hosted matrix, PR87 is ready for normal review and merge.

@TheHiddenObserver TheHiddenObserver left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Independent code review: BLOCKING FINDINGS

Audit mode under .claude/skills/code-review.md, against exact head 60071b02b0fcb5c51111b52cf3facbbc4a0df48b. GitHub does not permit the PR author to submit REQUEST_CHANGES on their own PR, so this is recorded as a COMMENT review; the technical verdict is request changes.

Active axes: backend, finite-input boundaries, formula lifecycle, estimator/sklearn API, solver/penalty compile paths, compatibility matrix, performance, and artifacts.

[HIGH][FORMULA][open] statgpu/_base.py:335-353 — finite validation is history-dependent after formula use.

Impact: formula_active is true whenever the estimator still has _design_info, even when the current call supplies direct X/y with formula=None and data=None. A model fitted once through a formula therefore skips pandas finite checks on a later direct refit/score. The direct fit body clears _design_info only after the guard has already run, so NaN/Inf can reach backend conversion or linear algebra and produce backend-specific errors or non-finite results instead of the standardized boundary error.

Fix: distinguish current-call formula ownership from stored prediction design state. Add a regression covering formula fit -> direct pandas refit/score with NaN and Inf.

[HIGH][API][open] statgpu/feature_selection/_stepwise.py:109-137, statgpu/feature_selection/_knockoff.py:260-430 — public feature-selection entry points remain outside the shared finite-input contract.

Impact: StepwiseSelector is publicly exported but is not a BaseEstimator; _prepare_X/_prepare_y only validate shape and it provides no finite guard or sklearn target tags. The public functional knockoff APIs convert X, y, and Xk directly with xp.asarray and do not call check_finite; only the selector classes are manually guarded. This violates Issue #81's acceptance criterion that all public numerical entry points either reject non-finite values consistently or explicitly document supported missing behavior. For the default correlation statistic, NaNs can propagate into W and yield a plausible empty selection rather than the standardized error.

Fix: add a shared manual contract for Stepwise and all three functional knockoff entry points, including requires_y tags for Stepwise, and include required-constructor estimators/functions in the public inventory.

[HIGH][MATRIX][open] .github/workflows/maintenance-compatibility.yml:16-23 — the required sklearn 1.3.x compatibility row is absent.

Impact: Issue #82 explicitly requires 1.2.2, 1.3.x, and latest. The new workflow pins only 1.2.2, while the general workflow installs an unpinned latest version. sklearn 1.3.x is the transition point for __sklearn_clone__, so neither legacy 1.2 nor latest coverage substitutes for an explicit 1.3 gate.

Fix: use a matrix with at least 1.2.2, 1.3.*, and latest, running clone, nested clone, Pipeline, GridSearchCV, tags, and the public-estimator inventory in each row.

[HIGH][MATRIX][open] dev/tests/test_maintenance_024_025.py:300-370, statgpu/solvers/_fista_lla.py:38-132, statgpu/linear_model/legacy/_elasticnet_legacy.py:177-203 — physical GPU evidence does not execute all changed compiled solver call sites.

Impact: the hardware matrix exercises a generic compiled function, one public Lasso fit, and direct penalty proximal calls. Direct proximal calls do not exercise the compiled fused LLA/FISTA kernels, solver convergence/KKT behavior, or the legacy ElasticNet compiled proximal path. Issue #45 enumerates 11 affected call sites, and the repository's solver gate requires smooth, nonsmooth, nonconvex, group, adaptive, convergence/KKT, and backend evidence.

Fix: add model-level modern-CUDA tests for ElasticNet and representative SCAD/MCP/group LLA paths that force the affected solver kernels, assert compiled diagnostics without fallback, and record convergence/objective or KKT evidence.

[HIGH][MATRIX][open] dev/tests/test_maintenance_024_025.py:80-105, 365-390 — Issue #81's three-backend metamorphic/device-purity criterion is not demonstrated at public estimator boundaries.

Impact: the test named test_torch_finite_validation_stays_on_device constructs a CPU tensor, and the physical Torch matrix does not run a CUDA finite-validation test. The CuPy hardware test validates the helper only. There is no common public estimator entry point tested with the same NaN/Inf cases on NumPy, CuPy, and Torch CUDA, so backend-specific guard installation and standardized exception behavior can still diverge.

Fix: parameterize at least one representative public fit/predict/inference boundary across NumPy, CuPy, and Torch CUDA; assert original device preservation, scalar-only synchronization, exception type, and message parity.

[MEDIUM][API][open] statgpu/_base.py:977-1043set_params() is described as transactional but nested updates mutate shared child objects before commit.

Impact: fresh = type(self)(**direct) receives the original estimator-valued parameter object. Nested set_params() therefore mutates that shared child before self.__dict__ is replaced; the current test explicitly observes the original child changing. With multiple nested roots, a later failure can leave earlier children partially mutated, contradicting the transactional contract.

Fix: clone estimator-valued direct parameters before applying nested updates, or narrow/document the guarantee as transactional only for direct parameters and add a partial-failure regression.

[MEDIUM][PERF][needs optimization] compile mode changes remain deferred without the evidence required by the repository review policy.

Impact: the user-visible caveat exists, but there is no machine-readable benchmark, optimization_notes, warm-up/steady-state timing, memory result, or eager/default/reduce-overhead comparison. The code-review policy permits performance deferral only with benchmark evidence, optimization notes, and a caveat.

Fix: add a small results/*.json benchmark covering Lasso, ElasticNet, one nonconvex path, and one group path with synchronized Torch timings and explicit uncovered reasons where necessary.

[MEDIUM][ARTIFACT][needs user approval] PR #87 commit history — merge method must be constrained.

Impact: the PR contains 132 commits, including temporary bootstrap/write-enabled workflow commits that are deleted from the final tree. A regular merge would preserve that transient history on master.

Fix: squash merge (or rebuild a clean branch) rather than using merge-commit/rebase history preservation.

Positive evidence remains strong: hosted CI, packaging, the RTX 4090 compile matrix, and the P100 CuPy test all pass on the stated head. Those results do not close the formula-state, public-entry-point, compatibility, and solver-matrix gaps above.

@TheHiddenObserver
TheHiddenObserver force-pushed the agent/maintenance-0.2.4-0.2.5 branch from 5c2bf75 to 3834d01 Compare August 5, 2026 04:21

@TheHiddenObserver TheHiddenObserver left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Final local review/fix-loop result

Reviewed exact head fce4b8aed36f0fff500169c702d77a0eccbe14b3 under .claude/skills/code-review.md after completing the repeated auto-fix → targeted validation → independent re-review loop.

Latest independent review

No new local CRITICAL, HIGH, or in-scope MEDIUM finding was identified.

The final round rechecked:

  • formula/direct-array finite-validation ownership, including historical formula state and unrelated data= arguments;
  • function-style knockoff finite checks and Stepwise/knockoff supervised transformer contracts;
  • transactional direct and nested set_params() behavior, rollback, shared-child isolation, and fitted-state reset;
  • sklearn clone/tag/Pipeline/GridSearchCV behavior under 1.2.2, 1.3.2, and latest;
  • model-level Torch compile coverage for Elastic Net, SCAD, MCP, group SCAD, and group MCP;
  • compiled-callable cache isolation and observable no-fallback assertions;
  • the isolated eager-versus-compiled benchmark, including enforced prediction/coefficient precision thresholds;
  • removal of all temporary write-enabled review workflows from the final tree;
  • release-package and merge-history hygiene.

Exact-head hosted evidence

  • Tests #1126: success — full CPU suite 1927 passed, 674 skipped; static/docs and Python 3.9–3.12 jobs passed.
  • Maintenance compatibility #139: success under scikit-learn 1.2.2, 1.3.2, and latest; benchmark syntax passed in every job.
  • Release notes validation #153: success.
  • Release package validation #159: success, including clean sdist install and Ubuntu/Windows/macOS wheel smoke.

Remaining gate

REVIEW_CLEAN_REMOTE_GPU_PENDING

The local review/fix loop is complete. Current-head physical Torch/CuPy tests and dev/benchmarks/benchmark_torch_compile_maintenance.py have not yet been run. Prior GPU evidence from 60071b02... is not accepted for this head.

Because the branch history contains temporary workflow commits, this PR must be squash-merged only after the exact-head GPU gate passes.

Copy link
Copy Markdown
Owner Author

Final .claude/skills/code-review.md review-fix cycle completed on exact head 1c44fe7d1047c91b1a7abd5a65a48140fd81ef54.

  • Seven additional fix rounds were completed in this cycle.
  • The latest independent exact-tree review found no new local CRITICAL, HIGH, or MEDIUM findings.
  • Tests #1394 passed: 2238 passed, 719 skipped, 20 warnings; static/docs and Python 3.9–3.12 matrix all passed.
  • Maintenance compatibility #407, release notes #421, and release package #427 all passed, including Linux/macOS/Windows wheel smoke.
  • The final tree contains no temporary review workflow or CI trigger.

Status remains REVIEW_CLEAN_LOCAL_FULL_HOSTED_PASS_GPU_REVALIDATION_PENDING: backend/runtime and classifier behavior changed, so current-head physical Torch/CuPy validation is still required before merge readiness. The exact commands are recorded in the PR body. Squash merge only; no merge was performed.

@TheHiddenObserver TheHiddenObserver left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Final independent review/fix-loop result

Reviewed exact head 1c44fe7d1047c91b1a7abd5a65a48140fd81ef54 in auto-fix mode under .claude/skills/code-review.md.

Latest review verdict

No new CRITICAL, HIGH, or in-scope MEDIUM finding was identified. No additional source change was necessary in this pass.

The review covered two independent passes across the changed and adjacent runtime surface:

  • GLM/IRLS statistical objective, arbitrary Binomial links, analytic weights, warm starts, convergence publication, and penalty validation;
  • direct LogisticRegression state transactions, strict response/control validation, likelihood/metric semantics, robust inference, and backend-native prediction contracts;
  • penalized and dedicated Ridge/Logistic/ElasticNet CV candidate-failure classification, weighted grids/scoring, backend selection, final-refit device consistency, and publish-after-success state;
  • BaseEstimator raw/public versus normalized/private constructor state, recursive clone, transactional nested set_params, sklearn tags, and public finite-input guards;
  • formula-owned sample-weight alignment after row filtering, including Torch/CuPy device preservation;
  • sandwich and post-fit inference scaling, HC1/dispersion behavior, singular-system classification, and no masking of infrastructure/device errors;
  • smooth, nonsmooth, nonconvex, group, and adaptive solver dispatch plus centralized observable Torch compile fallback;
  • a horizontal scan of broad exception handlers, CPU/GPU fallback boundaries, and temporary review artifacts.

The initially suspected integer-weight normalization issue was traced through every affected solver path and is not reachable: weighted FISTA/FISTA-BB/Proximal-Newton convert weights to the design dtype/device before loss normalization, IRLS returns promoted validated weights, and Newton accepts only uniform weights whose global scale does not alter the averaged objective.

Exact-head evidence

  • source tree is identical to reviewed source head 1107cd4307c641ac29962a34f76977dfc74e5220; the two later commits have no file diff;
  • Tests #1394: success — 2238 passed, 719 skipped; static/docs and Python 3.9–3.12 jobs passed;
  • Maintenance compatibility #407: success;
  • Release notes validation #421: success;
  • Release package validation #427: success, including clean sdist and Linux/macOS/Windows wheel smoke tests;
  • no unresolved inline review threads remain.

Exit state

PARTIAL_REMOTE_PENDING

The review/fix loop is clean. The only remaining gate is the already documented exact-head physical Torch/CuPy revalidation; it is not a newly discovered source-review finding. Keep the PR unmerged until that gate passes, and squash-merge only because the branch history contains temporary review commits.

@TheHiddenObserver TheHiddenObserver left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Final review/fix-loop closure

Reviewed current head 3cefcb7ff7bd857c754e36059593b7d8b0286d81 in auto-fix mode under .claude/skills/code-review.md after incorporating the completed physical GPU gate and independently reviewing the reported import-order workaround.

[MEDIUM][MAINT][fixed] statgpu/losses/__init__.py — GLM imports depended on import order because eager Cox-loss export entered the survival package while glm_core._base was still initializing.

Impact: a clean process could require importing LogisticRegression before statgpu.glm_core internals. This was a real package-initialization defect even though it did not alter numerical results.

Fix: CoxPartialLikelihoodLoss is now a lazy public export. The public from statgpu.losses import CoxPartialLikelihoodLoss contract is preserved without recursively initializing survival models during the GLM base import.

Evidence: dev/tests/test_core_contracts.py::test_glm_core_import_order_is_clean_in_fresh_interpreter executes the previously problematic order in a new Python process and passes.

Final independent re-review

No new CRITICAL, HIGH, or in-scope MEDIUM finding remains. The final delta from the physical-GPU-tested head changes only:

  • statgpu/losses/__init__.py — package import ordering;
  • dev/tests/test_core_contracts.py — fresh-interpreter regression.

No backend kernel, solver, fitting, prediction, inference, or numerical implementation changed after the physical GPU run, so the final delta activates API/MAINT/TEST gates rather than the three-backend numerical gate.

Validation

Physical GPU evidence on 1c44fe7d1047c91b1a7abd5a65a48140fd81ef54:

  • RTX 4090 / PyTorch 2.8.0+cu128 compile and CUDA Graph matrix: 9/9 passed;
  • Torch IRLS and LogisticRegression runtime assertions: passed;
  • Tesla P100 / CuPy 13.6.0 IRLS and LogisticRegression runtime assertions: passed.

Current-head hosted evidence on 3cefcb7ff7bd857c754e36059593b7d8b0286d81:

  • Tests #1396: success — 2239 passed, 719 skipped; static/docs and Python 3.9–3.12 passed;
  • Maintenance compatibility #409: success under scikit-learn 1.2.2, 1.3.2, and latest;
  • Release notes validation #423: success;
  • Release package validation #429: success, including clean sdist and Linux/macOS/Windows wheel smoke.

No unresolved inline review thread remains.

Exit state

COMPLETE

PR 87 is review-complete and merge-ready. Because the branch history contains temporary review/validation commits, use squash merge only.

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.

2 participants