-
Notifications
You must be signed in to change notification settings - Fork 13
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Sequence building #1026
Sequence building #1026
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## 0.2 #1026 +/- ##
==========================================
+ Coverage 51.13% 51.47% +0.33%
==========================================
Files 56 56
Lines 2726 2745 +19
==========================================
+ Hits 1394 1413 +19
Misses 1332 1332
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
571faec
to
8f9248e
Compare
The |
1021273
to
746fe8f
Compare
b9e23ae
to
746fe8f
Compare
@@ -93,17 +94,36 @@ def pulse_channels(self, pulse_id: int) -> list[ChannelId]: | |||
"""Find channels on which a pulse with a given id plays.""" | |||
return [channel for channel, pulse in self if pulse.id == pulse_id] | |||
|
|||
def concatenate(self, other: "PulseSequence") -> None: | |||
def concatenate(self, other: Iterable[_Element]) -> None: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we accept any iterable here? Isn't PulseSequence
sufficient? Same for __or__
and __ior__
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
qibolab/tests/integration/test_sequence.py
Lines 22 to 29 in 746fe8f
seq = ( | |
q1.RX() | |
| p12.CZ() | |
| [(ch1.drive, Delay(duration=6.5))] | |
| q2.RX() | |
| q0.RX12() | |
| p02.CZ() | |
) |
The explicit pulse-like here (the Delay
) is contained in a list
, which is perfectly homogeneous to a PulseSequence
(since PulseSequence
is a list
subclass), but it's not a PulseSequence
.
In general, it is very cheap to support, and quite user-friendly (e.g. even an iterator over _Element
would work, so you can use literals such as list comprehension and generators, that otherwise should be manually wrapped in a PulseSequence
).
On my side, I'm ok to merge. But there are still two open discussions. |
2d6a018
to
53c1826
Compare
Pure syntactic sugar
Pure syntactic sugar
When concatenating two sequences Co-authored-by: Stavros Efthymiou <[email protected]>
53c1826
to
18ec55d
Compare
Just missing tests.