Add Python wrapper package#47
Conversation
|
@MilesCranmerBot please review this PR. In particular it would be useful to know if you think this is the right design. Also please check feature coverage of this. @gustavorubim thanks for your effort on this. Small initial comment is we probably want a different name for the PyPI package to indicate it is the rust backend. |
MilesCranmerBot
left a comment
There was a problem hiding this comment.
Thanks for putting this together — I think the wrapper-first direction is the right one for getting PySR onto a normal optional dependency instead of vendoring the Rust workspace. The narrow PyO3 boundary also seems sensible for an experimental backend.
That said, I would avoid publishing this API as stable until a few contract issues are fixed:
-
Blocker:
seedis accepted/documented but ignored.symbolic_regression_py/src/lib.rsmaps many option keys inapply_options, but never assignsoptions.seed, even though the README/tests pass"seed": 0and the Rust search usesOptions::seedfor RNG initialization. This will silently break reproducibility. -
Blocker: unknown options are silently ignored. Because
apply_optionsonly reads a whitelist, typos or PySR-forwarded-but-unsupported options become no-ops. For this boundary I’d strongly prefer a validated wire schema: either reject every unknown key, or report unsupported keys explicitly. Ideally this should reuse/generate from the existing Rust options spec /WasmOptionsShimpattern rather than maintaining a second manual mapping. -
Operator API shape is misleading. The Python API accepts
operators={2: [...]}, but the Rust code ignores the dict keys and flattens all values. Either take a flat list, or validate arity and exposeunary_operators/binary_operators-style fields. As written,{1: ["+"]}and{2: ["+"]}behave the same. -
Feature coverage is fine for an MVP, but should be explicit. The current wrapper covers dense float32/float64, single-output, builtin operators, scalar early-stop threshold, and a lot of scalar search options. Major PySR/Rust-engine features still missing include sample weights, non-MSE loss selection, per-variable/per-operator complexities, constraints/nested constraints, custom operators/losses, warm starts, and richer output. I’d call those unsupported loudly rather than allowing them through the options dict.
-
Package/API naming should be settled before release.
symbolic-regression-rsas the PyPI distribution does signal “Rust backend” reasonably well, but the distribution/import/crate/directory names are all slightly different. If the package is intended as a public standalone Python API, I’d add defaults and a small typed result layer. If it is mainly a PySR wire backend, I’d document the instability/private nature more explicitly. -
Release workflow: CI tests Ubuntu editable/wheel/sdist builds, but the publish workflow builds Windows/macOS/Linux wheels without smoke-testing each artifact before upload. I’d add at least an import + tiny search smoke test after each wheel build before publishing.
Overall: design direction 👍, but I’d request changes before merge/release for at least (1) seed propagation, (2) unknown-option rejection, and (3) a clearer operator/options contract. For feature coverage, the current subset is OK as a staged MVP if PySR’s adapter rejects unsupported features up front.
Local checks I ran:
cargo fmt --check -p symbolic_regression_rs_pycargo check -p symbolic_regression_rs_py- editable install into a fresh uv venv +
python -m unittest discover symbolic_regression_py/tests
Upstream check observed: pre-commit.ci is passing.
8b2a2bd to
b565624
Compare
|
Updated this PR in response to the package naming and contract feedback:
Validation rerun:
I also opened the in-PySR connector comparison draft here: astroautomata/PySR#1251 |
Summary
symbolic_regression_pyPyO3/maturin package exposingsymbolic_regression_rs.search(...).pysr-rust-backendwhile keeping the import modulesymbolic_regression_rsfor the PySR backend boundary.Context
This is the wrapper-first step for exposing the Rust engine to PySR as discussed in astroautomata/PySR#1197. Once this package is accepted and published, PySR can depend on a normal
pysr-rust-backendpackage from its optional Rust extra instead of vendoring the Rust workspace.Follow-up to review comments
seedoption intoOptions::seed.mutation_weightskeys instead of silently ignoring them.operators={arity: [names]}mapping.Related PR
Validation
cargo fmt --check -p symbolic_regression_rs_pycargo check -p symbolic_regression_rs_pycargo testpython -m pytest symbolic_regression_py/tests -q: 12 passed.github/workflows/ci-python-wrapper.ymland.github/workflows/publish-python-wrapper.ymlwith PyYAML