|
| 1 | +# LogisticRegressionCV Torch strict-CUDA repair plan |
| 2 | + |
| 3 | +Issue: #112 |
| 4 | +Baseline: `master` at `37f643a68ded71a33f8eea5ed8217aab42c650e1` |
| 5 | +Scope: correctness repair before the next roadmap feature package (#93) |
| 6 | + |
| 7 | +## 1. Why this is the next stage |
| 8 | + |
| 9 | +The benchmark/dashboard sequence through PR #115 is complete. The roadmap normally moves next to Panel P1 (#93), but its own priority rules put correctness and public-contract risk ahead of feature breadth. Canonical CV evidence currently records one maintained backend defect: `LogisticRegressionCV` fails on Torch strict CUDA while the corresponding NumPy, CuPy, and sklearn rows succeed. Therefore #112 is treated as a bounded correctness blocker before starting #93. |
| 10 | + |
| 11 | +This change does not reopen dashboard performance work and does not add a new statistical feature. |
| 12 | + |
| 13 | +## 2. Impact classification |
| 14 | + |
| 15 | +Active gates: |
| 16 | + |
| 17 | +- backend/device/dtype locality; |
| 18 | +- cross-validation; |
| 19 | +- numerical correctness/convergence; |
| 20 | +- tests and compatibility; |
| 21 | +- user-facing documentation/changelog; |
| 22 | +- physical-GPU validation. |
| 23 | + |
| 24 | +Inactive gates: |
| 25 | + |
| 26 | +- public constructor/API shape: no public arguments or return types should change; |
| 27 | +- formula semantics: no formula/model-matrix path changes are planned; |
| 28 | +- inference definition: final-refit inference contract is unchanged; |
| 29 | +- benchmark/performance claims: no speedup claim is planned. |
| 30 | + |
| 31 | +Capability decision: |
| 32 | + |
| 33 | +- backend: `three-backend` (existing capability, Torch path repaired); |
| 34 | +- CV: `supported`; |
| 35 | +- inference: `supported` for the final refit exactly as before; |
| 36 | +- formula: unchanged; |
| 37 | +- benchmark: correctness evidence required, performance claim not required. |
| 38 | + |
| 39 | +## 3. Root cause established from current source |
| 40 | + |
| 41 | +`_select_logistic_c_cv()` defaults to `gpu_cv_mixed_precision=True`, so the GPU CV working design is `float32`. |
| 42 | + |
| 43 | +In `_solve_logistic_path_gpu_from_batch()` the current Torch path then creates: |
| 44 | + |
| 45 | +- `params = backend.zeros(...)` without a dtype, and |
| 46 | +- `reg_diag = backend.full(...)` without a dtype. |
| 47 | + |
| 48 | +`TorchBackend.zeros/full` default to `torch.float64`. The first matrix multiply therefore combines a `float32` design matrix with `float64` parameters. Torch requires matching matrix-multiplication dtypes, so strict Torch CUDA fails before completing the first candidate. CuPy is more permissive about mixed-dtype arithmetic, which explains the backend-specific disposition seen in the canonical benchmark. |
| 49 | + |
| 50 | +The same helper also converts every fitted coefficient vector/intercept to NumPy inside the candidate/fold loop and then converts them back to the GPU backend for scoring. This does not cause the reported failure, but it is an unnecessary device round trip in the exact code being repaired and increases the risk of dtype drift. |
| 51 | + |
| 52 | +A repository-wide call-site search found exactly one caller of `_solve_logistic_path_gpu_from_batch()`: `_select_logistic_c_cv()` in the same module. Therefore changing this private helper to keep its return batches backend-native is a bounded internal refactor, not a public API change. Both `TorchBackend` and `CuPyBackend` provide the required `stack`, `copy`, `zeros`, and dtype-aware array-creation primitives. |
| 53 | + |
| 54 | +## 4. Implementation plan |
| 55 | + |
| 56 | +### A. Make the batched IRLS working dtype explicit |
| 57 | + |
| 58 | +In `statgpu/linear_model/cv/_logistic_cv.py`: |
| 59 | + |
| 60 | +- create `params` with `dtype=X_design.dtype`; |
| 61 | +- create regularization diagonals with `dtype=XtWX.dtype`; |
| 62 | +- keep coefficient/intercept batches in backend-native arrays through scoring; |
| 63 | +- use backend-native stacking so the returned path batch preserves dtype and device; |
| 64 | +- only transfer the per-fold loss vector to NumPy when populating the public `loss_path` result; |
| 65 | +- preserve the current objective, intercept non-penalization rule, folds, C grid, sample-weight semantics, convergence tolerance, and strict no-CPU-fallback behavior. |
| 66 | + |
| 67 | +No statistical definition changes are allowed in this repair. |
| 68 | + |
| 69 | +### B. Add deterministic regression coverage that does not require hosted CUDA |
| 70 | + |
| 71 | +Add focused tests using `TorchBackend(device="cpu")` to exercise the internal batched helper on both working-dtype modes: |
| 72 | + |
| 73 | +- default mixed precision: `float32`; |
| 74 | +- `gpu_cv_mixed_precision=False` analogue: `float64`. |
| 75 | + |
| 76 | +The tests should prove: |
| 77 | + |
| 78 | +- the helper no longer raises a Float/Double dtype mismatch; |
| 79 | +- returned coefficient/intercept batches remain Torch tensors on the selected backend and keep the requested working dtype; |
| 80 | +- candidate outputs are finite and have the expected shapes; |
| 81 | +- the path solver itself performs no `backend.to_numpy()` conversion; |
| 82 | +- float32 and float64 paths agree to a documented mixed-precision tolerance on the same deterministic problem. |
| 83 | + |
| 84 | +Add/extend public CV tests to preserve: |
| 85 | + |
| 86 | +- CPU/CuPy/Torch selection semantics where the backend is available; |
| 87 | +- explicit-device no-fallback behavior; |
| 88 | +- sample-weight behavior; |
| 89 | +- deterministic candidate selection and transactional final refit. |
| 90 | + |
| 91 | +The private return-type change is safe only while the single-caller invariant holds; the regression should exercise that consuming path rather than expose the helper publicly. |
| 92 | + |
| 93 | +### C. Check compatibility and numerical parity |
| 94 | + |
| 95 | +Run the strongest locally available evidence: |
| 96 | + |
| 97 | +- targeted LogisticRegressionCV tests; |
| 98 | +- Torch CPU internal float32/float64 regressions; |
| 99 | +- maintenance/static tests covering CV backend routing; |
| 100 | +- full hosted CPU/compatibility workflows on the PR head. |
| 101 | + |
| 102 | +Numerical checks should compare the same C grid/folds and ensure the repaired path does not alter the declared loss or selection semantics beyond normal mixed-precision tolerance. |
| 103 | + |
| 104 | +### D. Documentation and issue state |
| 105 | + |
| 106 | +Update the root/EN/CN changelogs with the repaired strict-Torch CV behavior once implementation is validated. |
| 107 | + |
| 108 | +Do **not** edit the canonical `2026-08-07` benchmark source from `failed` to `success` without a new physical-GPU measurement. Historical measured evidence remains immutable. |
| 109 | + |
| 110 | +Issue #112 should close only after a current-source physical Torch CUDA reproduction succeeds. If physical GPU access is unavailable in this run, finish with `PARTIAL_REMOTE_PENDING` and leave #112 open with the exact rerun command/evidence needed. |
| 111 | + |
| 112 | +## 5. Physical-GPU acceptance |
| 113 | + |
| 114 | +Required for `COMPLETE`: |
| 115 | + |
| 116 | +- Torch CUDA on a real NVIDIA GPU, preferably the original Tesla P100 / Torch 2.0.x compatibility environment or an equally strict supported environment; |
| 117 | +- run the canonical CV reproduction for `LogisticRegressionCV` with default `gpu_cv_mixed_precision=True`; |
| 118 | +- also smoke the `gpu_cv_mixed_precision=False` path on the same source head; |
| 119 | +- confirm strict Torch execution succeeds without CPU fallback; |
| 120 | +- compare selected C / CV loss against NumPy or CuPy within a documented mixed-precision tolerance; |
| 121 | +- record software/hardware/source SHA and the exact command. |
| 122 | + |
| 123 | +A fresh canonical benchmark artifact may be added only from an actual rerun. The old failed artifact remains historical evidence. |
| 124 | + |
| 125 | +## 6. Plan review/fix closure criteria |
| 126 | + |
| 127 | +Review the plan before implementation for: |
| 128 | + |
| 129 | +1. whether the proposed fix changes any statistical definition; |
| 130 | +2. whether it accidentally weakens strict device semantics; |
| 131 | +3. whether it leaves another Torch mixed-dtype boundary in fit or scoring; |
| 132 | +4. whether tests can catch the original failure on CPU-only CI; |
| 133 | +5. whether both float32 and float64 working modes are covered; |
| 134 | +6. whether the private backend-native return change has any caller outside this module; |
| 135 | +7. whether the plan improperly rewrites historical benchmark evidence; |
| 136 | +8. whether the change should remain bounded to #112 rather than becoming a general CV refactor. |
| 137 | + |
| 138 | +First plan review findings and fixes: |
| 139 | + |
| 140 | +- **MEDIUM / TEST — fixed:** the initial plan tested only the default float32 path. It now requires both float32 and float64 working modes plus cross-dtype numerical parity. |
| 141 | +- **MEDIUM / MAINT — fixed:** the initial plan changed the private helper return representation without establishing call-site scope. Repository search confirms exactly one caller in the same module, and CuPy/Torch backend primitives support the bounded backend-native stacking change. |
| 142 | + |
| 143 | +Implementation may begin only after a fresh re-review finds no new in-scope plan issue. |
| 144 | + |
| 145 | +After implementation, run `.claude/skills/code-review.md` in auto-fix mode and repeat targeted validation/re-review until no new CRITICAL/HIGH or in-scope MEDIUM issue is found. |
| 146 | + |
| 147 | +## 7. Non-goals |
| 148 | + |
| 149 | +- no Panel #93 implementation in the same PR; |
| 150 | +- no general `_penalized_cv.py` decomposition; |
| 151 | +- no solver rewrite; |
| 152 | +- no new penalty, formula, or inference behavior; |
| 153 | +- no dashboard optimization or schema change; |
| 154 | +- no fabricated or reconstructed benchmark success row; |
| 155 | +- no performance claim without a new measured artifact. |
| 156 | + |
| 157 | +## 8. Expected exit |
| 158 | + |
| 159 | +- `COMPLETE` only if local/hosted gates and physical Torch CUDA validation all pass; |
| 160 | +- otherwise `PARTIAL_REMOTE_PENDING` is acceptable when the implementation/review is clean and only physical-GPU evidence is missing. |
0 commit comments