Skip to content

(2/3) Add a method to generate a list of circuits for TREX benchmarking and readout calibration - #8185

Merged
mhucka merged 17 commits into
quantumlib:mainfrom
ddddddanni:trex_second
Aug 21, 2026
Merged

(2/3) Add a method to generate a list of circuits for TREX benchmarking and readout calibration#8185
mhucka merged 17 commits into
quantumlib:mainfrom
ddddddanni:trex_second

Conversation

@ddddddanni

@ddddddanni ddddddanni commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator

This PR adds a method generate_trex_and_readout_circuits which generates pauli circuits and readout circuits for trex readout benchmarking and pauli calculation.

The generated circuits are tested in: https://colab.sandbox.google.com/drive/1LVOEUDIkx5EtFJmCSyh2UR6QP_3t-PDG?resourcekey=0-O3gWlDvxUUWDx23O2xqtgQ#scrollTo=UNCUryvsqUfO, which looks good.

@github-actions github-actions Bot added the size: M 50< lines changed <250 label Jul 8, 2026
@codecov

codecov Bot commented Jul 8, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.59%. Comparing base (4f8bf1b) to head (93cc212).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #8185      +/-   ##
==========================================
- Coverage   99.59%   99.59%   -0.01%     
==========================================
  Files        1122     1122              
  Lines      102566   102598      +32     
==========================================
+ Hits       102154   102182      +28     
- Misses        412      416       +4     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@ddddddanni
ddddddanni marked this pull request as ready for review July 8, 2026 21:45
@ddddddanni
ddddddanni requested a review from a team as a code owner July 8, 2026 21:45
@ddddddanni

Copy link
Copy Markdown
Collaborator Author

@NoureldinYosri and @obriente can you take a look at this one?

@mhucka

mhucka commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

@ddddddanni it looks like there's a type annotation check failure in CI. when you get a chance, could you investigate?

@ddddddanni

Copy link
Copy Markdown
Collaborator Author

@ddddddanni it looks like there's a type annotation check failure in CI. when you get a chance, could you investigate?

Done! I will also find someone to review this PR next week.

@mhucka

mhucka commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

@ddddddanni Thank you for your work!

I went ahead and added Nour and Tom to the reviewers for this PR, per your comment a couple of weeks ago.

@mhucka
mhucka requested review from NoureldinYosri and obriente and removed request for shatterednirvana July 20, 2026 05:13

@obriente obriente left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Some minor text edit requests, and one big question re: data output format.

@ddddddanni
ddddddanni requested a review from obriente July 26, 2026 23:02

@obriente obriente left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM, new formatting makes sense, thanks for fixing the nits.

@mhucka mhucka 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.

@ddddddanni Thank you for this work! I have one smallish request. the generate_trex_and_readout_circuits function has an insert_strategy parameter, but it doesn't look like the tests exercise the possibility that the value is not INLINE. Could the tests be expanded to cover that?

This might be a place where a parametrized test is useful. Something like

@pytest.mark.parametrize(
    "insert_strategy, expected_moments_count",
    [
        (cirq.InsertStrategy.INLINE, 3),
        (cirq.InsertStrategy.EARLIEST, 2),
    ],
)
def test_generate_trex_and_readout_circuits_insert_strategy(
    insert_strategy: cirq.InsertStrategy, expected_moments_count: int
) -> None:
    """Test that the insert_strategy is correctly applied to generated twirled circuits."""

    ...

@mhucka mhucka self-assigned this Jul 28, 2026
@ddddddanni

Copy link
Copy Markdown
Collaborator Author

@ddddddanni Thank you for this work! I have one smallish request. the generate_trex_and_readout_circuits function has an insert_strategy parameter, but it doesn't look like the tests exercise the possibility that the value is not INLINE. Could the tests be expanded to cover that?

This might be a place where a parametrized test is useful. Something like

@pytest.mark.parametrize(
    "insert_strategy, expected_moments_count",
    [
        (cirq.InsertStrategy.INLINE, 3),
        (cirq.InsertStrategy.EARLIEST, 2),
    ],
)
def test_generate_trex_and_readout_circuits_insert_strategy(
    insert_strategy: cirq.InsertStrategy, expected_moments_count: int
) -> None:
    """Test that the insert_strategy is correctly applied to generated twirled circuits."""

    ...

Done!

@ddddddanni
ddddddanni requested a review from mhucka July 29, 2026 04:30
@mhucka

mhucka commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Done!

Thank you!

@mhucka mhucka 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.

@ddddddanni A couple of possible issues may be lurking.

@github-actions github-actions Bot added size: L 250< lines changed <1000 and removed size: M 50< lines changed <250 labels Aug 2, 2026

@mhucka mhucka 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.

I'm sorry to have a couple more issues spotted in a final look-over … (I know this has gone on too long. This will be the last changes.)

q: qubit_pauli_dict_unsorted[q] for q in sorted(qubit_pauli_dict_unsorted)
}

joint_basis_pauli: cirq.PauliString = ops.PauliString(qubit_pauli_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.

I'm not sure why this switches between cirq.PauliString and ops.PauliString. Should they be the same? It looks like the rest of the code usually uses ops, so maybe the type declaration should be:

Suggested change
joint_basis_pauli: cirq.PauliString = ops.PauliString(qubit_pauli_dict)
joint_basis_pauli: ops.PauliString = ops.PauliString(qubit_pauli_dict)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Yes those are the same, and I realize I mess up with the two. Changed!

@ddddddanni
ddddddanni requested a review from mhucka August 19, 2026 00:45

@mhucka mhucka 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

@mhucka
mhucka enabled auto-merge August 21, 2026 01:58
@mhucka
mhucka added this pull request to the merge queue Aug 21, 2026
Merged via the queue into quantumlib:main with commit e0b54ff Aug 21, 2026
48 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/calibration area/performance size: L 250< lines changed <1000 status/awaiting-response Waiting on a response from someone

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants