Resolve shared CondaPkg NumPy conflict with CIMOptimizer - #56
Conversation
bernalde
left a comment
There was a problem hiding this comment.
Reviewing as maintainer at head 8d02eff. I am the PR author, so GitHub permits only a COMMENT event from me; a formal approve/request-changes must come from another maintainer. There are no blocking issues; on the merits this is an approve.
This PR removes DWave's direct PyPI numpy = "~=2.2.0" pin (added in #53) so the shared CondaPkg solve can use the conda-selected NumPy, updates the metadata tests, and adds test/shared_condapkg_resolve.jl plus a README section. Verified against merge-base 93b634c: 4 files, 67 additions, 11 deletions, matching the PR totals.
Intent vs issue #55: the change is the right fix for the right problem. #55 reports that the ~=2.2.0 PyPI pin (numpy <2.3) conflicts with the conda-side numpy 2.4.6 that CIMOptimizer pulls via pytorch-cpu. Removing the pin is option 1 from the issue. I verified both acceptance criteria by reproducing the resolves myself with CondaPkg v0.2.36:
- DWave (no numpy) + CIMOptimizer: resolves, conda selects numpy 2.4.6.
- Full tier DWave + PySA + CIMOptimizer + QiskitOpt: resolves, numpy 2.4.6, qiskit 2.3.1.
I also confirmed the three hardcoded tier snapshots in the metadata test match the real registeredCondaPkg.tomlof QiskitOpt v0.7.1, PySA v0.4.2, and CIMOptimizer v0.2.2. Notably QiskitOpt v0.7.1 itself dropped its numpy pin, so with DWave also dropping its pin no package now constrains numpy below 2.3; that is what makes the full tier resolve.Closes #55is appropriate. DWave's source only uses stable numpy APIs (np.array,from_numpy_vectors), so deferring to the conda-selected numpy carries no API-break risk.
Blocking issues:
- None.
Nonblocking issues:
test/compat_metadata.jlcontains a tautological assertion:Test.@test haskey(cimoptimizer_conda_deps, "pytorch-cpu")checks a literal dict the test itself just defined, so it always passes and verifies nothing about DWave or real CIMOptimizer metadata. Either drop it or replace it with an assertion that exercises something real (e.g. that DWave declares no condapytorch/numpy that would collide).- The snapshot comment was weakened relative to #53. It now reads
Mirrors the registered Python-backed benchmark tier from issue #55and no longer names the mirrored versions, whereas the prior comment namedQiskitOpt v0.7.0. The versions (CIMOptimizer 0.2.2, QiskitOpt 0.7.1, PySA 0.4.2) live only intest/shared_condapkg_resolve.jl. Name them in the comment too so the hardcoded snapshot can be re-audited deliberately when the tier moves.
Questions:
- The NumPy policy has now flip-flopped across PRs (removed, then pinned
~=2.2.0in #53, now removed again), each time driven by a new driver's constraint. Theaudited stable stacktestset now asserts!haskey(pip_deps, "numpy"), i.e. numpy is intentionally no longer audited and floats to whatever ocean-sdk/conda selects. That is reasonable given CIMOptimizer's conda numpy, and the!haskeytest does guard against a future re-add. Should the README or a comment state this policy explicitly (DWave intentionally does not pin numpy and defers to dwave-ocean-sdk plus the shared conda solve), so the next contributor does not reintroduce a pin and restart the cycle? Non-blocking; the test already encodes the rule, but the rationale is undocumented.
Tests run:
- Ran the two updated testsets in
test/compat_metadata.jlstandalone against headCondaPkg.toml: both pass (audited 4/4, shared benchmark env 7/7). - Reproduced acceptance criterion 1 (DWave + CIMOptimizer) and criterion 2 (full tier) via
CondaPkg.resolve()on manually-merged CondaPkg.toml files built from the real registered dependency sets: both resolve, numpy 2.4.6. - Did not run the full
Pkg.test()suite (imports DWave through PythonCall, needs the full Python env and, for the cloud sampler, network/credentials) nor the heavytest/shared_condapkg_resolve.jlend-to-end through the Julia registry; the dependency resolution it checks is verified above, and the PR reports both green.
Merge-readiness: Mergeable. The fix is correct and both issue acceptance criteria are verified; the nonblocking items are test-quality and documentation polish. I defer the formal approval to another maintainer since I authored this PR.
|
Commits pushed:
Main changes:
Tests:
Comments intentionally not addressed:
Declined Blocking comments:
Remaining risks or follow-up items:
|
Summary
test/shared_condapkg_resolve.jland README documentation for the full Python-backed JuliaQUBO benchmark-tier CondaPkg resolve smoke.Closes #55.
Tests
julia --project=. -e 'include("test/compat_metadata.jl")'env JULIA_DEPOT_PATH=/tmp/dwave-issue55-pair/depot JULIA_PKG_PRECOMPILE_AUTO=0 julia --startup-file=no --project=/tmp/dwave-issue55-pair/project -e 'import Pkg; Pkg.Registry.update(); Pkg.develop(Pkg.PackageSpec(path=pwd())); Pkg.add([Pkg.PackageSpec(name="CIMOptimizer", version=v"0.2.2"), Pkg.PackageSpec(name="CondaPkg", version=v"0.2.36")]); import CondaPkg; CondaPkg.resolve(); println("DWave + CIMOptimizer CondaPkg resolve succeeded")'env JULIA_DEPOT_PATH=/tmp/dwave-issue55-full/depot JULIA_PKG_PRECOMPILE_AUTO=0 julia --startup-file=no test/shared_condapkg_resolve.jljulia --project=. -e 'import Pkg; Pkg.test()'Branch Hygiene
mainfix/issue-55-shared-condapkg-numpyorigin/main