Skip to content

Cr sequences - #994

Closed
jevillegasdatTII wants to merge 568 commits into
cross_resonancefrom
cr_sequences
Closed

Cr sequences#994
jevillegasdatTII wants to merge 568 commits into
cross_resonancefrom
cr_sequences

Conversation

@jevillegasdatTII

@jevillegasdatTII jevillegasdatTII commented Oct 2, 2024

Copy link
Copy Markdown
Contributor

Checklist:

  • Reviewers confirm new code works as expected.
  • Tests are passing.
  • Coverage does not decrease.
  • Documentation is updated.
  • Compatibility with Qibo modules (Please edit this section if the current pull request is not compatible with the following branches).
    • Qibo: master
    • Qibolab: 0.1
    • Qibolab_platforms_qrc: main

Closed previous pull request #987 and make this one with a change in the sequenced routine.

@codecov

codecov Bot commented Oct 2, 2024

Copy link
Copy Markdown

Codecov Report

Attention: Patch coverage is 72.85714% with 114 lines in your changes missing coverage. Please review.

Project coverage is 96.39%. Comparing base (0a8b3a4) to head (a1d134b).
Report is 5 commits behind head on cross_resonance.

Files with missing lines Patch % Lines
...t_interaction/cross_resonance/chevron_frequency.py 49.45% 46 Missing ⚠️
...ction/cross_resonance/cross_resonance_sequences.py 38.57% 43 Missing ⚠️
...ols/two_qubit_interaction/cross_resonance/utils.py 0.00% 25 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@                 Coverage Diff                 @@
##           cross_resonance     #994      +/-   ##
===================================================
- Coverage            97.53%   96.39%   -1.15%     
===================================================
  Files                  123      129       +6     
  Lines                 9703    10072     +369     
===================================================
+ Hits                  9464     9709     +245     
- Misses                 239      363     +124     
Flag Coverage Δ
unittests 96.39% <72.85%> (-1.15%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
src/qibocal/protocols/__init__.py 100.00% <100.00%> (ø)
src/qibocal/protocols/ramsey/ramsey.py 99.00% <100.00%> (-0.09%) ⬇️
src/qibocal/protocols/ramsey/ramsey_signal.py 97.54% <100.00%> (-0.10%) ⬇️
src/qibocal/protocols/ramsey/utils.py 100.00% <100.00%> (ø)
src/qibocal/protocols/state_tomography.py 98.65% <100.00%> (+0.10%) ⬆️
...ibocal/protocols/two_qubit_interaction/__init__.py 100.00% <100.00%> (ø)
...protocols/two_qubit_interaction/chevron/chevron.py 97.05% <ø> (-2.21%) ⬇️
.../two_qubit_interaction/cross_resonance/__init__.py 100.00% <100.00%> (ø)
...s/two_qubit_interaction/cross_resonance/chevron.py 100.00% <100.00%> (ø)
...bit_interaction/cross_resonance/cross_resonance.py 100.00% <100.00%> (ø)
... and 4 more

... and 3 files with indirect coverage changes

@jevillegasdatTII

Copy link
Copy Markdown
Contributor Author

Results in #987 (comment)

@Edoardo-Pedicillo
Edoardo-Pedicillo self-requested a review October 2, 2024 12:33

@Edoardo-Pedicillo Edoardo-Pedicillo 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.

Thanks @jevillegasdatTII. Before merging to main, could you update the protocol section of the Qibocal documentation (https://qibo.science/qibocal/stable/protocols/index.html) with a description of these routines, following the same style? You can check here https://github.com/qiboteam/qibocal/tree/main/doc/source/protocols  for some examples.

@dataclass
class CrossResonanceChevronParameters(Parameters):
"""ResonatorSpectroscopy runcard inputs."""
"""cross resonance chevron runcard inputs."""

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.

Start the docstrings with the capital letter

Comment on lines +40 to +45
0(I):
Q_C: Pulse(omega_T, t) - MZ
Q_T: wait - MZ
1(X):
Q_C: RX - Pulse(omega_T, t) - MZ
Q_T: - wait - MZ

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 suggest explaining the pulse sequence in a sentence, because it could create possible rendering issues and some symbols are not defined.

@dataclass
class CrossResonanceSeqData(Data):
"""Data structure for Cross Resonance Gate Calibration using Sequences.
targets: [target, control]

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.

Suggested change
targets: [target, control]

Comment on lines +54 to +55
from qibolab.pulses import Pulse, Rectangular, PulseType, Gaussian
from qibolab.native import NativePulse

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.

Move these lines on top

phi = results[qubit].phase
data.register_qubit(
CrossResonanceType,
(qubit, target, control, tgt_setup, ctr_setup),

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.

Why do you need to store qubit?

from qibolab.platform import Platform
from qibolab.qubits import QubitId, QubitPairId

def cr_pulse_sequence(platform: Platform, pair: QubitPairId, setup: tuple, duration:int=0, amplitude:int = None):

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 function is not used, but (I didn't check carefully) it could be used in cross_resonance and the signal one to avoid code duplication.

@Edoardo-Pedicillo Edoardo-Pedicillo added this to the Qibocal 0.1.1 milestone Oct 3, 2024
jevillegasd and others added 27 commits February 15, 2025 09:52
Bumps [jinja2](https://github.com/pallets/jinja) from 3.1.4 to 3.1.5.
- [Release notes](https://github.com/pallets/jinja/releases)
- [Changelog](https://github.com/pallets/jinja/blob/main/CHANGES.rst)
- [Commits](pallets/jinja@3.1.4...3.1.5)

---
updated-dependencies:
- dependency-name: jinja2
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <support@github.com>
chore(deps): bump jinja2 from 3.1.4 to 3.1.5
This reverts commit b5d0417, reversing
changes made to 2343e0a.
…ccept to select in which bases to measure and the setup of the target qubit.
@andrea-pasquale

Copy link
Copy Markdown
Contributor

@jevillegasdatTII is this PR still necessary? Seems to be quite outdated with respect to main. Also keep in mind that for CR there are already several protocols tested by @Luca-Ben-Herrmann in #860. I suggest to close it and keep the branch in case we want something compatible with 0.1 just for debugging.

@jevillegasd jevillegasd closed this Feb 4, 2026
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.