Skip to content

refactor(jmle): rebase Rust optimizer ownership contract on current main - #744

Closed
seonghobae wants to merge 1 commit into
mainfrom
replacement/jmle-rust-optimizer-main-20260811-v2
Closed

refactor(jmle): rebase Rust optimizer ownership contract on current main#744
seonghobae wants to merge 1 commit into
mainfrom
replacement/jmle-rust-optimizer-main-20260811-v2

Conversation

@seonghobae

Copy link
Copy Markdown
Contributor

Controlled clean-lineage replacement for #738 / issue #626

Protected main at creation is 933ce6cdfedbbf92b30e73ed25f0ef0d926357f2. Old Draft #738 is now stale and its exact writer handoffs were explicitly bound to predecessor bases. This replacement preserves its only unique delta—the fail-first JMLE optimizer-ownership contract—without transferring predecessor checks, reviews, approvals or mergeability.

Intentional RED

The first commit adds only tests/test_jmle_rust_optimizer_ownership.py. It requires installed public JMLE fitting under backend="rust" to complete for adam, lbfgs, and adam_lbfgs while sentinels replace legacy Python _adam / _lbfgs loops.

Valid RED must build/import the actual Rust/PyO3 package and reach optimizer ownership. Setup/import/fixture/changelog failure is not evidence.

Minimum GREEN

Move result-affecting JMLE optimizer arithmetic/control into mlsirm-core: Adam state/update and gradient clipping; L-BFGS history/two-loop/curvature/line-search; and adam_lbfgs phase sequencing/convergence needed to preserve the public result. Expose thin PyO3 transport; Python keeps validation, deterministic marshalling and reporting only. Missing/incompatible Rust capability must fail closed. Preserve public optimizer/model/seed/restart/objective/trace semantics unless a separate test-first scientific defect proves otherwise.

Add direct Rust/public parity and realistic aligned bias/MAE/RMSE/convergence recovery, complete docs/rustdoc, APA-7 method traceability and changelog only after GREEN. Keep CPU execution unless profiling justifies GPU. Require one unchanged final head with focused/full Python 3.12/3.14, Rust/PyO3, package/reinstall, existing GPU/fuzz, Security/SAST, exact coverage/docstrings and current-head review/governance gates.

Do not widen into MMLE, providers, dependencies/workflows/version/release, hosted state, or canonical #604 documentation. Advances #626 and supersedes #738 once this replacement is established.

@coderabbitai

coderabbitai Bot commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 7e9bf84d-7e0c-4f1b-aa02-2b79e478d7fb

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Copy link
Copy Markdown
Contributor Author

@opencode-agent address

Exact bounded implementation handoff for issue #626 / Draft #744. Re-read source head 48d050d046362d011e2e85bf2b55c3a57d5e88f1 and independently resolve protected main; proceed only while live main remains 933ce6cdfedbbf92b30e73ed25f0ef0d926357f2 and no other source writer has moved or claimed this branch. Preserve the fail-first commit/history; no force-push, destructive rebase, temporary writer workflow, documentation side branch, or unrelated cleanup.

RED is exact and production-boundary-valid on CI run 31515549519: editable Rust/PyO3 build and Rust-primary backend verification succeeded; Rust workspace, package/reinstall/release acceptance, GPU smoke, fuzz, Security Scan and SAST succeeded; Python 3.12 reached the full suite and failed only the three new ownership cases (adam, lbfgs, adam_lbfgs) because public JMLE executes Python _adam / _lbfgs arithmetic. Do not weaken/delete/skip those tests.

Implement the narrowest GREEN that makes Rust the production optimizer owner while preserving current public behavior:

  1. Move Adam update/state/gradient clipping and L-BFGS history/two-loop/curvature/line-search plus adam_lbfgs phase sequencing/convergence into mlsirm-core; prefer one explicit Rust optimizer entrypoint/interface over Python callbacks that retain result-affecting arithmetic.
  2. Expose thin PyO3 marshalling. Python may validate inputs/config, initialize/marshal deterministic state, select declared optimizer mode, and construct/report results; it must not retain an independent production Adam/L-BFGS numerical implementation as a fallback. Missing/incompatible core capability fails closed.
  3. Preserve current FitConfig optimizer names, model/objective semantics, restart/seed behavior, iteration accounting/status/trace fields and numeric tolerances unless a separate realistic RED proves a defect. Do not touch MMLE.
  4. Add direct Rust/Python field parity and focused Rust tests for Adam, L-BFGS and combined sequencing, including non-finite/line-search/curvature/convergence edge behavior already encoded by current Python tests. Preserve existing JMLE recovery; add/strengthen aligned bias/MAE/RMSE/convergence evidence where ownership changes expose a gap. Correlation alone is insufficient.
  5. CPU-f64 only for this slice unless profiling demonstrates material GPU benefit. Add complete rustdoc/public docs, APA-7 optimizer source traceability and a changelog fragment only after GREEN.
  6. Before any Ready transition: focused GREEN, complete relevant Python 3.12/3.14, Rust/PyO3, package/reinstall, existing GPU/fuzz, Security/SAST, exact coverage/docstrings, current-head automated review and repository governance on one unchanged head.

Stop writing immediately if source/base/writer identity changes. Update the PR body with exact new evidence; keep Draft while any gate remains.

Copy link
Copy Markdown
Contributor Author

Superseded by clean current-main Draft #751. Fresh compare proved #744 head 48d050d046362d011e2e85bf2b55c3a57d5e88f1 is four protected-main commits behind and its only unique path is tests/test_jmle_rust_optimizer_ownership.py. The exact prior writer handoff was explicitly conditional on protected main remaining 933ce6...; that condition is false, so it is stale. #751 recreates the exact ownership RED from protected main 8a56d7b1b024e18a5a96a94db4ede1bf6648ead1 on head 386fe8ca52ba9def121e050993cdb5317f37c184. No predecessor checks/reviews/approval transfer.

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.

1 participant