qhc 1073 add phase to iqpair drag and prepare for future improvements

[jordivallsq]

As Fabio suggested and to make measurements more straightforward iq pair needs to include a phase rotation. As this is a complex subject, for now we can implement simply a 90 degrees rotation and set a not implemented error for the rest.

[github-actions]

Hello. You may have forgotten to update the changelog!
Please edit changelog-dev.md with:

• A one-to-two sentence description of the change. You may include a small working example for new features.
• A link back to this PR.

[pullpo-for-slack]

🔗Pullpo.io Slack PR-Channel

[pullpo-for-slack]

AI Analysis

Purpose of the changes:

Add phase handling to IQPair class for improved waveform manipulation.

Improvements and suggestions

(dropdown):

• In file src/qililab/waveforms/iq_pair.py / __post_init__

Issue:
The phase handling logic does not correctly swap the I and Q waveforms when the phase is 90 degrees; it uses deepcopy incorrectly, making both I and Q the same.

Suggested solution:
Instead of using deepcopy, directly assign Q to I and I to Q when the phase is 90 degrees. This avoids unnecessary copying of the same object.

• In file src/qililab/waveforms/iq_pair.py / DRAG

Issue:
The DRAG method currently lacks validation for the phase parameter, which may lead to unexpected behavior if an invalid phase is passed.

Suggested solution:
Add validation for the phase parameter in the DRAG method to ensure it only accepts 0 or 90 degrees.

Detailed file changes

(dropdown):

• In file src/qililab/waveforms/iq_pair.py:
  • Introduced a phase attribute to the IQPair class to manage phase differences between I and Q waveforms.
  • Enhanced the post_init method to handle phase values, raising an error for unsupported phases.
  • Modified the DRAG method to accept a phase parameter, allowing for more flexible waveform creation.

[codecov]

Codecov Report

❌ Patch coverage is 70.00000% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 97.87%. Comparing base (846db4a) to head (c7f8ef8).

Files with missing lines	Patch %	Lines
src/qililab/waveforms/iq_pair.py	70.00%	3 Missing ⚠️

❌ Your patch check has failed because the patch coverage (70.00%) is below the target coverage (100.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #978      +/-   ##
==========================================
- Coverage   97.89%   97.87%   -0.03%     
==========================================
  Files         239      239              
  Lines       10468    10476       +8     
==========================================
+ Hits        10248    10253       +5     
- Misses        220      223       +3     

Flag	Coverage Δ
unittests	97.87% <70.00%> (-0.03%)	⬇️

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

☔ View full report in Codecov by Sentry.
📢 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.

[sonarqubecloud]

Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[fedonman]

I completely disagree with adding a phase attribute in waveform level for many reasons.

1. Phase is not an intrinsic attribute of a waveform or an IQ pair, the physically relevant phase in experiments is a frame-level concept that composes across pulses and measurements rather than belonging to a single waveform object.

2. Phase is already handled at a higher level in our native Rmw/DRAG gates, where it is explicitly defined and can be realized either by an equivalent I/Q envelope rotation or, more commonly, by a Virtual-Z (frame) update, so duplicating it at the waveform level is unnecessary and semantically incorrect.

3. If we need to rotate an I/Q pair we can do so by creating a utility function that uses the common formula: I′(t)Q′(t)​=I(t)cosϕ−Q(t)sinϕ=I(t)sinϕ+Q(t)cosϕ​

4. The current code is buggy e.g.

self.I = deepcopy(self.Q)
self.Q = deepcopy(self.I)

After the first line, self.I is already overwritten, so the second line deepcopies the new self.I (which equals the old Q). Result: both I and Q become the same waveform.

[jordivallsq]

This PR was created to add an extra variable to IQ Pair for the calibration file. As Fabio pointed out, to create a waveform inside Calibration we should allow the user to rotate the IQPair in a physical level without needing the user to perform complex functions. I agree that we might put this implementation somewhere else, but on the other hand, This makes the implementation inside the calibration file much simpler (in the yaml, instead of showing a huge arbitrary wf, it is a smaller text).

Regarding your points:

1. This would be a rotation inside the IQ plane. To answer your 1st point, the phase is not an intrinsic atribute of the IQPair but is an atribute of the IQ plane and right now is handled in a digital level, not modifying the envelope of the pulses sent.
2. For digital, Rmw gates including phase and Virtual rotations are working well, but for calibration of dQPU we will not be using those gates.
3. This is basically the function to be applied but ideally I would like to maintain the waveform type of some rotations (specially square and flat top pulses as they are optimized in qblox.
4. I agree that this is poorly implemented, buggy and needs a complete update (right now, as you say, it doesn't work). This PR was a proof of concept for fabio to test as defined in this linear task. We can make this way better. for this reason I will be changing it to a draft.

Another important reason why this PR was blocked, is the new functions for IQPair (IQPair.DRAG was making things more complex but now it is simpler to implement).

[sonarqubecloud]

Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[fedonman]

I just saw it was created a long time ago, sorry I thought it was something urgent brought up.

It's true that Rmw is digital-only. But pulse programming already supports IQ rotation: Either with the set_phase() (recommended way) or manually rotating the IQWaveform (we can add a utility method if it's more user friendly)

For the calibration file we can edit it to include rotation, I have no issues with that. But it should be a new data structure specifically for the calibration file and not change the IQWaveform. E.g. it could be

class FrameWaveform:
   waveform: IQWaveform
   phase: float

or something similar. But again it would be more "correct" to implement with SetPhase() + Play() operations. The issue with rotating the IQWaveform is that many envelopes are created and the memory can overflow, whereas by using SetPhase() you could have unlimited rotations using the same envelope.

[jordivallsq]

It seems like a good solution but I need to check the best way to implement something similar. I agree that the right way is to implement similar with set phase but my idea was to modify the envelope before reaching qblox compilation, this way we avoid unnecessary waveforms