Skip to content

Sorts gate array to have deterministic gate selection#16139

Open
amruthpremjith wants to merge 3 commits intoQiskit:mainfrom
amruthpremjith:random-cliff-array-sort
Open

Sorts gate array to have deterministic gate selection#16139
amruthpremjith wants to merge 3 commits intoQiskit:mainfrom
amruthpremjith:random-cliff-array-sort

Conversation

@amruthpremjith
Copy link
Copy Markdown

AI/LLM disclosure

  • I didn't use LLM tooling, or only used it privately.
  • I used the following tool to help write this PR description:
  • I used the following tool to generate or modify code:

Fix #16138

Sort the gate array before selecting from it to return the same gates for the same set of inputs.

Tested with and without changes.

from qiskit.circuit.random.utils import random_clifford_circuit

cliff_circuit = random_clifford_circuit(num_qubits=10, num_gates=100, gates="all", seed=0)
print(cliff_circuit.count_ops())

Without change

(venv) amruthpremjith@MacBookPro qiskit % python3 test-6.py 
OrderedDict({'x': 10, 'h': 10, 'iswap': 9, 'cz': 9, 'z': 9, 's': 8, 'cy': 8, 'sx': 6, 'ecr': 6, 'cx': 6, 'dcx': 4, 'swap': 4, 'sdg': 4, 'y': 3, 'id': 3, 'sxdg': 1})
(venv) amruthpremjith@MacBookPro qiskit % python3 test-6.py
OrderedDict({'h': 10, 'sxdg': 10, 'iswap': 9, 'cz': 9, 'sdg': 9, 'z': 8, 'cy': 8, 'x': 6, 'ecr': 6, 'cx': 6, 'dcx': 4, 'swap': 4, 'sx': 4, 'id': 3, 'y': 3, 's': 1})

With change

(venv) amruthpremjith@MacBookPro qiskit % python3 test-6.py
OrderedDict({'z': 10, 'x': 10, 'ecr': 9, 'cy': 9, 'h': 9, 'sx': 8, 'cz': 8, 'id': 6, 'iswap': 6, 'cx': 6, 'swap': 4, 'dcx': 4, 'sxdg': 4, 's': 3, 'y': 3, 'sdg': 1})
(venv) amruthpremjith@MacBookPro qiskit % python3 test-6.py
OrderedDict({'z': 10, 'x': 10, 'ecr': 9, 'cy': 9, 'h': 9, 'sx': 8, 'cz': 8, 'id': 6, 'iswap': 6, 'cx': 6, 'swap': 4, 'dcx': 4, 'sxdg': 4, 's': 3, 'y': 3, 'sdg': 1})

@amruthpremjith amruthpremjith requested a review from a team as a code owner May 5, 2026 11:42
@amruthpremjith amruthpremjith requested a review from jakelishman May 5, 2026 11:42
@qiskit-bot qiskit-bot added the Community PR PRs from contributors that are not 'members' of the Qiskit repo label May 5, 2026
@qiskit-bot
Copy link
Copy Markdown
Collaborator

Thank you for opening a new pull request.

Before your PR can be merged it will first need to pass continuous integration tests and be reviewed. Sometimes the review process can be slow, so please be patient.

While you're waiting, please feel free to review other open PRs. While only a subset of people are authorized to approve pull requests for merging, everyone is encouraged to review open pull requests. Doing reviews helps reduce the burden on the core team and helps make the project's code better for everyone.

One or more of the following people are relevant to this code:

  • @Qiskit/terra-core

@alexanderivrii
Copy link
Copy Markdown
Member

Thanks! That was fast. Do you think there is any meaningful test we could add? Could you also add a (short) bugfix release note?

@alexanderivrii alexanderivrii added the Changelog: Fixed Add a "Fixed" entry in the GitHub Release changelog. label May 5, 2026
@amruthpremjith
Copy link
Copy Markdown
Author

amruthpremjith commented May 5, 2026

Sure, maybe we can add a test case that runs it twice and compares the gate count?

@jakelishman
Copy link
Copy Markdown
Member

The randomisation of set with string keys is fixed by PYTHONHASHSEED, which is only read/set once during initialisation of the Python interpreter, so just doing the same thing twice in the same process would pass without this change. We can test either by comparing a decent-sized circuit to a "blessed" value, or by spawning separate interpreters with different PYTHONHASHSEEDs and ensuring they come back the same.

Comment thread qiskit/circuit/random/utils.py Outdated
Comment on lines +711 to +712
gates_1q = sorted(set(_BASIS_1Q.keys()) - {"v", "w", "id", "iden", "sinv"})
gates_2q = sorted(_BASIS_2Q.keys())
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorted is perhaps a bit unnecessarily heavy as a normalisation step; dict is already consistently ordered (including the dict_keys iterator), so the sort of gates_2q isn't needed, and the top line can be something along the lines of

ignore_1q = {"v", "w", "id", "iden", "sinv"}
gates_1q = [gate for gate in _BASIS_1Q if gate not in ignore_1q]

and avoid the extra $\lg n$. (Not that this would be performance critical, but still.)

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I updated it to the snippet you mentioned, I was thinking it was straightforward to just sort it it to be consistent across any changes in the source dictionary as well.

@alexanderivrii
Copy link
Copy Markdown
Member

alexanderivrii commented May 5, 2026

Lol, Jake beat me at replying :)

@coveralls
Copy link
Copy Markdown

Coverage Report for CI Build 25374288175

Coverage decreased (-0.01%) to 87.587%

Details

  • Coverage decreased (-0.01%) from the base build.
  • Patch coverage: 2 of 2 lines across 1 file are fully covered (100%).
  • 20 coverage regressions across 4 files.

Uncovered Changes

No uncovered changes found.

Coverage Regressions

20 previously-covered lines in 4 files lost coverage.

File Lines Losing Coverage Coverage
crates/circuit/src/parameter/symbol_expr.rs 8 73.93%
crates/qasm2/src/parse.rs 6 97.63%
crates/qasm2/src/lex.rs 5 91.77%
crates/circuit/src/parameter/parameter_expression.rs 1 90.53%

Coverage Stats

Coverage Status
Relevant Lines: 121826
Covered Lines: 106704
Line Coverage: 87.59%
Coverage Strength: 963202.31 hits per line

💛 - Coveralls

@amruthpremjith
Copy link
Copy Markdown
Author

I added a test case that passes in different seeds to a subprocess, without the change the test case failed and it was passing with it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Changelog: Fixed Add a "Fixed" entry in the GitHub Release changelog. Community PR PRs from contributors that are not 'members' of the Qiskit repo

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

The function random_clifford_circuit should yield deterministic results on each program run.

5 participants