Skip to content

Feature/symmetric stabilizer benchmarking#1164

Merged
vietphamngoc merged 27 commits into
mainfrom
feature/symmetric_stabilizer_benchmarking
Feb 26, 2026
Merged

Feature/symmetric stabilizer benchmarking#1164
vietphamngoc merged 27 commits into
mainfrom
feature/symmetric_stabilizer_benchmarking

Conversation

@cdbf1

@cdbf1 cdbf1 commented Mar 10, 2025

Copy link
Copy Markdown
Contributor

Add symmetric stabilizer benchmarking to the QCVV library.

@richrines1 - this is based off my initial branch (as I wasn't sure where yours ended up) so feel free to suggest changes based on your experience!

Closes #1069

@cdbf1 cdbf1 linked an issue Mar 10, 2025 that may be closed by this pull request
@review-notebook-app

Copy link
Copy Markdown

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@cdbf1 cdbf1 marked this pull request as ready for review March 11, 2025 17:27

@dowusu-antwi dowusu-antwi left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM % a couple of nits

Comment on lines +38 to +44
assert len(experiment._stabilizer_states) == 12
assert len(experiment._init_rotations) == 12
for rot in experiment._init_rotations:
assert len(rot) == 5
assert len(experiment._reconciliation_rotation) == 12
for rot in experiment._reconciliation_rotation:
assert len(rot) == 4

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can we add a comment describing how these follow from the experiment parameters?

Comment thread supermarq-benchmarks/supermarq/qcvv/ssb.py Outdated
@richrines1

Copy link
Copy Markdown
Contributor

@cdbf1 my version is here

most of the changes are probably superfluous/now outdated, but i think the important ones are:

  • updating the build_circuits implementation to determine reconciliation circuits a lot more efficiently
  • dropping the initial XX in _sss_init_circuits (this was used in the original paper, but not necessary)
  • the addition of placeholders between adjacent layers of single-qubit gates (though these should probably just be barriers, not CZ.with_tags("placeholder")

Comment thread supermarq-benchmarks/supermarq/qcvv/ssb.py Outdated
Comment on lines +336 to +337
assert num == depth - 2, (max_depth, depth, num)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

this can be removed - it's not really checking anything

Suggested change
assert num == depth - 2, (max_depth, depth, num)

Comment thread supermarq-benchmarks/supermarq/qcvv/ssb.py Outdated
Comment on lines +401 to +409
def _json_dict_(self) -> dict[str, Any]:
"""Converts the experiment to a json-able dictionary that can be used to recreate the
experiment object. Note that the state of the random number generator is not stored.

Returns:
Json-able dictionary of the experiment data.
"""
return super()._json_dict_()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

should this be adding something? otherwise it doesn't seem necessary to override

Suggested change
def _json_dict_(self) -> dict[str, Any]:
"""Converts the experiment to a json-able dictionary that can be used to recreate the
experiment object. Note that the state of the random number generator is not stored.
Returns:
Json-able dictionary of the experiment data.
"""
return super()._json_dict_()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

_json_dict is declared as an abstract method for the parent class to force a custom implementation when necessary.

num = 0
for i in range(max_depth - 2):
if i in indices:
circuit += cirq.CZ.on(*self.qubits).with_tags("no_compile")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

do we need to add barriers around this like we did in #1199?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Added barriers just to ensure the compiler is not simplifying the different blocks.

Comment thread supermarq-benchmarks/supermarq/qcvv/ssb.py Outdated
Comment thread supermarq-benchmarks/supermarq/qcvv/ssb.py Outdated

@dowusu-antwi dowusu-antwi left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nits + error fixes:

Comment thread supermarq-benchmarks/supermarq/qcvv/ssb.py Outdated
Comment thread supermarq-benchmarks/supermarq/qcvv/ssb.py Outdated
Comment thread supermarq-benchmarks/supermarq/qcvv/ssb.py
Comment thread supermarq-benchmarks/supermarq/qcvv/ssb_test.py Outdated
Comment thread supermarq-benchmarks/supermarq/qcvv/ssb_test.py Outdated
Comment thread supermarq-benchmarks/supermarq/qcvv/ssb_test.py Outdated
@cdbf1

cdbf1 commented Feb 26, 2026

Copy link
Copy Markdown
Contributor Author

LGTM (although GH won't let me review as I originally opened the PR nearly a year ago...)

@vietphamngoc vietphamngoc merged commit 92425fa into main Feb 26, 2026
21 checks passed
@vietphamngoc vietphamngoc deleted the feature/symmetric_stabilizer_benchmarking branch February 26, 2026 09:23
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.

symmetric stabilizer benchmarking

4 participants