Qhc 1358 Remove ext_trigger check from platform - #1112
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1112 +/- ##
=======================================
Coverage 98.71% 98.71%
=======================================
Files 209 209
Lines 11306 11308 +2
=======================================
+ Hits 11161 11163 +2
Misses 145 145
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
fedonman
left a comment
fedonman
left a comment
There was a problem hiding this comment.
Made a comment on the location of the flag.
flavie-lebars
left a comment
flavie-lebars
left a comment
There was a problem hiding this comment.
Why do we even need the flag in the runcard? Another architecture could be that as soon as a qprogram has a wait_trigger we set up whatever needs to be set up between the compilation and execution. The downisde would be that this would need to be done at runtime (but this would be negligible) but this would avoid the user needing to put this flag and make things simpler in my opinion.
Also, I completely agree with Vyron's comment.
jordivallsq
changed the title
|
jordivallsq
left a comment
jordivallsq
left a comment
There was a problem hiding this comment.
@flavie-lebars @fedonman Following your advice, the external_trigger flag in the runcard has been removed, now the device creates this external trigger inside the trigger network when wait_trigger is used.
The downisde would be that this would need to be done at runtime (but this would be negligible) but this would avoid the user needing to put this flag and make things simpler in my opinion.
I need to test this on hardware to see if the qblox controller has any reaction to run
self.device.ext_trigger_input_trigger_en(True)
self.device.ext_trigger_input_trigger_address(15)
self.device.ext_trigger_input_delay(0)
many consecutive times (I don't expect anything but just in case).
new changelog
Removed external_trigger parameter from within the runcard's qblox controller instrument. Now the function QbloxClusterController.set_ext_trigger is risen internally every time a qprogram contains a wait_trigger using the trigger channel 15 (last one).
| self.qprogram = qprogram | ||
| self.sequences = sequences | ||
| self.acquisitions = acquisitions | ||
| self.external_trigger = external_trigger |
There was a problem hiding this comment.
in my opinion, the flag shouldn't be inferred in the compiler (_handle_wait_trigger), it's known at qprogram-authoring time. We already have this pattern: trigger_network_required lives on QProgram._QbloxInterface, set inside measure_reset(). external_trigger should follow this. But I think @fedonman's comment suggested to move it here in the compiler. Happy to discuss this at a sprint/daily.
There was a problem hiding this comment.
I have implemented these changes in a single commit (Implemented requested changes) so the changes are reversible. Let's see what @fedonman thinks of the changes.
jordivallsq
added
the
needs-testing-on-qblox
|
3 participants
Moved qblox external trigger check from compiler to platform. This avoids raising errors related to the Runcard inside the compiler.