Qiskit interop: warning hygiene, 3.0-proofing, honest dependency floors#839
Merged
Conversation
Promote QiskitInteropWarning to an error in pytest.ini so interop concerns
surface as test failures.
Centralize the qiskit version check in a new pygsti.tools._qiskit_interop
helper with a tested version *range* (currently [1.4, 3)) instead of ten
copy-pasted exact-match checks against 2.1.1. The helper also removes the
warn-inside-try/bare-except pattern that converted the promoted warning into
a false "Qiskit is required ... does not appear to be installed" RuntimeError,
the MissingDependencyWarning misclassification in subcircuit_selection, and
the NameError fall-through in internalgates when qiskit is truly absent.
Other warning fixes:
- from_qiskit warns about discarded classical data only when the circuit
actually has clbits, not for the empty 'cr' register convert_to_qiskit
always attaches, so measurement-free round trips are now silent.
- The plain-UserWarning 'skipping measure' becomes a QiskitInteropWarning
with a real message.
- from_qiskit gains a lossy={'warn','ignore','raise'} parameter; the
mirror-edesign pipeline passes lossy='ignore' where loss is by design.
- fullstack mirror edesign no longer forwards coupling_map to transpile()
alongside a backend (the warning said it was ignored but it actually
overrode the backend's map); ignored-arg warnings now fire once, up front.
- simple_weighted_subcirc_selection keeps its qiskit module binding (the old
bare-except silently disabled qiskit-native instruction-duration lookups)
and no longer converts a CouplingMap type error into a bogus ImportError.
Add warning-path unit tests in test/unit/tools/test_qiskit_interop.py.
Remove all uses of qiskit APIs that are deprecated for removal in qiskit 3.0, plus remaining private-API usage: - mirror_edesign.get_active_qubits_no_dag: stop iterating CircuitInstruction as a tuple (removed in 3.0); use the .operation/.qubits attributes. - mirror_edesign ancilla detection: replace qubit._index and layout.initial_layout[idx]._register.name == 'ancilla' with the public circuit.find_bit(qubit).index and isinstance(..., AncillaQubit). - Replace circuit._qbit_argument_conversion(i)[0] with circuit.qubits[i] (public and stable since qiskit 1.0) in mirror_edesign; the circuit.py sites were converted in the previous commit. - Tests: replace the deprecated QFT class (removed in 3.0, and built on the also-removed BlueprintCircuit) with QFTGate appended to a QuantumCircuit. With this, the only deprecation warning left in the qiskit test files fires inside qiskit-ibm-runtime's own plugin loader, not from pyGSTi code. A global promote-qiskit-DeprecationWarnings pytest filter was considered (plan phase 2) but not added: warning module attribution follows the warn() stacklevel, so such a filter would have to promote DeprecationWarnings attributed to pygsti modules generally, which risks unrelated failures across the full suite.
qiskit-ibm-runtime's ConvertToMidCircuitMeasure transpiler pass only exists in runtime >=0.44 (Dec 2025), but it was imported in the same try/except as the core runtime names, so on any older runtime the whole block failed and _Sampler was set to None -- disabling all IBMQ submission even though nothing else required the new pass. Import it under its own guard and assert only in the code path that actually converts mid-circuit measurements (with a message that names the real fix: upgrade runtime or pass use_ibm_mcm=False). Also delete the stale qiskit-1.1.1/runtime-0.25.0 era comment block, narrow the bare excepts to ImportError, and update the 'ibmq' extra in pyproject.toml from the stale/unbounded 'qiskit>1, qiskit-ibm-runtime>=0.17.1' to 'qiskit>=1.4,<3, qiskit-ibm-runtime>=0.40', matching TESTED_QISKIT_RANGE in pygsti.tools._qiskit_interop. Validated: all 11 ibmq tests pass on qiskit 2.5.0 + runtime 0.47 (py313) and now also on qiskit 2.1.1 + runtime 0.41.1, where they previously failed solely because of the coupled import.
…(phase 4) - test_scarab.py: smoke tests for all three scarab benchmark wrappers (lowlevel/fullstack/subcircuit_mirror_benchmark), which previously had zero test coverage despite being exported from pygsti.protocols. These would have caught the phase-1 bare-except bug and will catch future signature drift. - test_circuit.py: convert_to_qiskit argument matrix -- qubits_to_measure None/'all'/'active'/list, num_qubits larger than the circuit width, metadata ordered_data_indices, and the invalid-option error path. - test_subcirc_selection.py: greedy_growth_subcirc_selection (direct call with return_depth_info, and via sample_subcircuits strategy='greedy') plus a stochastic_2q_drops test. The stochastic_2q_drops test is marked expectedFailure: exercising the previously-uncovered path revealed it is broken -- retained dangling gates have support outside the selected qubit subset, so building the subcircuit with line_labels=sorted(qubit_subset) always raises ValueError. Recorded as finding 7 in the active-project findings; the fix needs a maintainer decision (expand line labels vs truncate dangling gates). Per the plan, extras/devices gets no new tests; deprecation is proposed to the maintainers instead (its calibration formats predate the runtime API).
Runs the qiskit-facing test files against three environments: bare qiskit 1.4 (conversion floor, runtime-dependent tests skip), the declared runtime floor (qiskit-ibm-runtime 0.40 + qiskit<3), and whatever pip resolves today. Keeps TESTED_QISKIT_RANGE continuously honest and flags qiskit 3.0 breakage the week it ships rather than at user-report time. Local cross-version validation of the full change set, all with the strict error::QiskitInteropWarning filter and all 141 passed + 2 xfailed: - qiskit 2.5.0 + qiskit-ibm-runtime 0.47 (py313) - qiskit 2.1.1 + qiskit-ibm-runtime 0.41.1 - qiskit 1.4.6 + qiskit-ibm-runtime 0.37.0
nkoskelo
reviewed
Jul 14, 2026
rileyjmurray
marked this pull request as ready for review
July 15, 2026 02:59
sserita
approved these changes
Jul 15, 2026
sserita
left a comment
Contributor
There was a problem hiding this comment.
+1 on Nick's suggestion for the subseq for easier selection of tests, but not blocking. Other than that, looks great, merge as you will. I'll make an issue for the 2q_drops problem, which will be on my plate most likely.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This makes pyGSTi's qiskit interop clean under
error::pygsti.tools.exceptions.QiskitInteropWarning, which this PR promotes from a defined-but-inert exception class to apytest.inierror. Getting there required fixing several real bugs that were hiding behind warnings nobody could see (or that were misclassified so they wouldn't fire at all), removing qiskit APIs slated for removal in 3.0, and correcting theibmqextra's dependency floors, which were stale enough to break on qiskit versions we claim to support.Five commits, one per phase, each leaving the suite green under the strict filter:
7e39339dc→73a4f54bc→f4178a8c3→962c67a1e→1bb2ae93f.What changed
Warning hygiene (
pygsti/tools/_qiskit_interop.py, new)qiskit==2.1.1, scattered acrosscircuit.py,internalgates.py,subcircuit_selection.py,mirror_edesign.py, andscarab.py, are replaced by one helper,check_qiskit_version(context, required=True), checked against a tested range (TESTED_QISKIT_RANGE = ((1, 4), (3,))) instead of a single pin.try/bare-exceptthat turned the promoted warning into a false "qiskit does not appear to be installed"RuntimeError; aMissingDependencyWarningmisclassification insubcircuit_selection.pywhere a genuine version mismatch was reported as a missing dependency and execution silently continued; and aNameErrorfall-through ininternalgates.pywhen qiskit is truly absent.from_qiskitnow warns about discarded classical data only when the circuit actually has clbits — previously it fired onlen(circuit.cregs), so a pure pygsti→qiskit→pygsti round trip warned about discarding the emptycrregister thatconvert_to_qiskitalways attaches, even when nothing was measured.UserWarning'skipping measure'is now aQiskitInteropWarningwith an actual message.from_qiskitgainslossy={'warn', 'ignore', 'raise'}; the mirror-edesign pipeline passeslossy='ignore'at call sites where the loss is intentional.mirror_edesign's fullstack path no longer forwardscoupling_maptotranspile()alongside a backend — the warning claimed it was ignored, but an explicitcoupling_mapactually overrides the backend's map, so the old behavior silently used the wrong topology.test/unit/tools/test_qiskit_interop.pyexercises the warning paths directly.Qiskit 3.0-proofing
CircuitInstructionas a tuple (→.operation/.qubits),qubit._indexandlayout.initial_layout[idx]._register.name == 'ancilla'(→circuit.find_bit(qubit).indexandisinstance(..., AncillaQubit)), andcircuit._qbit_argument_conversion(i)[0](→ the public, stable-since-1.0circuit.qubits[i]).QFTclass (built on the also-removedBlueprintCircuit) is replaced withQFTGate.DeprecationWarnings to errors. Warning attribution followsstacklevel, so such a filter would promote all pygsti-attributedDeprecationWarnings, not just qiskit-related ones — too much blast radius on the full suite. Documented in the phase-2 commit message.extras/ibmqguard split + honest floorsibmqexperiment.pyimportedConvertToMidCircuitMeasure(added inqiskit-ibm-runtime0.44) in the sametry/exceptas the core runtime names. On any older runtime the whole import failed and_Samplerwas set toNone, disabling all IBMQ submission — even though nothing else needed the new pass. It's now guarded separately, with the assertion only in the code path that actually converts mid-circuit measurements (message names the real fix: upgrade runtime, or passuse_ibm_mcm=False).pyproject.toml'sibmqextra goes from the stale, unboundedqiskit>1, qiskit-ibm-runtime>=0.17.1toqiskit>=1.4,<3, qiskit-ibm-runtime>=0.40, matchingTESTED_QISKIT_RANGE.Coverage
test/unit/protocols/test_scarab.py(new): smoke tests for all three scarab benchmark wrappers (lowlevel/fullstack/subcircuit_mirror_benchmark), which are public API inpygsti.protocols.scarabbut previously had zero coverage — these would have caught the phase-1 bare-except bug.test_circuit.py: aconvert_to_qiskitargument matrix (qubits_to_measureinNone/'all'/'active'/list form, oversizednum_qubits,ordered_data_indicesmetadata, the invalid-option error path).test_subcirc_selection.py:greedy_growth_subcirc_selectioncoverage (direct call and viasample_subcircuits(strategy='greedy')), plus astochastic_2q_dropstest — see below.subcircuit_selection.pycoverage goes from 42% to 80%.extras/devicesgets no new tests, per plan: its calibration formats predate the current runtime API, and deprecation is proposed to maintainers instead of investing in coverage.CI
.github/workflows/qiskit-interop.yml, weekly: a bare qiskit-1.4-floor matrix leg (runtime uninstalled), a runtime-0.40-floor leg, and a latest leg. KeepsTESTED_QISKIT_RANGEhonest and should flag qiskit 3.0 breakage the week it ships rather than at user-report time.Reviewer attention
stochastic_2q_drops=Trueis broken. Writing thegreedy/stochasticcoverage in phase 4 (previously 0% covered) surfaced thatsimple_weighted_subcirc_selection(..., stochastic_2q_drops=True)re-adds dangling two-qubit gates whose support, by definition, includes at least one qubit outside the selected subset — and the subcircuit is then built withline_labels=sorted(qubit_subset), soCircuit.__init__raisesValueErroron essentially any connected circuit. The new test documents the intended behavior and is marked@unittest.expectedFailure. Fixing it needs a maintainer call: should retained dangling gates expand the subcircuit's line labels (changing its effective width), or should they be truncated to their in-subset qubits? Whoever picks a direction should also remove theexpectedFailuremarker.ibmqextra's dependency floor moved fromqiskit>1toqiskit>=1.4,<3and fromqiskit-ibm-runtime>=0.17.1to>=0.40. This is a correction, not a new restriction — anything outside the old, unbounded range was already broken (that's how finding 5 was found), but it's technically a tightening of the declared floor that downstream pins might notice.DeprecationWarningpromotion. See the qiskit-3.0-proofing section above — flagging so it isn't independently proposed and then hit the same blast-radius problem.Testing
Cross-version validation, all with the strict
error::QiskitInteropWarningfilter, 141 passed + 2 xfailed on each: