Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
feat!: introduce ExecutionDependency::Scheduled #186
feat!: introduce ExecutionDependency::Scheduled #186
Changes from 2 commits
5357a0d
7d757eb
f3e96db
4643052
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
A query on the spec; wouldn't a HALT have a precise timing? ie. you would know precisely when the program ends?
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.
You make a good point, perhaps about the docs or choice of names.
Many instructions will have timing fixed at compile time, but whether a halt is fixed to time X or time Y does not materially affect the program, if we see its pulse sequence as essential to its purpose.
Put another way, depending on the semantics of a hardware provider, the HALT may be 10 ns or 10us or 10ms after the previous instruction, but that does not affect the readout results that one would expect to collect.
That’s in contrast to a PULSE, where durations and times may be statically (if parametrically) determined from the quil alone, and which do not depend on hardware provider implementation details.
How could I better communicate that, maybe in docs but ideally in the code itself?
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.
I think the documentation. As a user, I never write HALT; it's always implicit, and if the hardware back end chooses to add it for some tracking of the type you describe, so be it. The place I would care is Quil-T based pulse timing analysis. One of the things we can do with Quil-T entirely on the user side is mimic pulse timing rules and determine the timing of all of the instructions ourselves. Here, we report total program duration as the end time of the last pulse. But, where HALT is in play (implicitly or explicitly) and has some real time duration as you describe, that real time should be accounted for (start time of HALT was the end time of the last pulse, but then there is some real end time to HALT that is greater?). Which brings me back to the point-- wouldn't you be able to tell me precisely what that time was? If the back end adds 10 microseconds, shouldn't I be able to see that somehow? Is this was "precise" means here, and what is
quil-rs
's part to play in this workflow (it may be it is simply out of scope at this level).This was a query of the semantics only, and not a practical issue, so I'm happy whatever you decide.
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.
Thanks for that feedback, @mhodson-rigetti !
Following up from our discussion offline, I've renamed this method to
is_scheduled
, andExecutionDependency::Timing
toExecutionDependency::Scheduled
.For anyone following along - "precise timing" vs scheduling is a worthwhile semantic distinction here; only some instructions are considered for "scheduling" as part of the program, and their position within that schedule is essential to the program doing what the author intended. That is now what this function identifies.
All instructions, though, may have "precise times" fixed as part of compilation for a backend. The timing of non-scheduled instructions (like
ADD
orMOVE
) is flexible within some bounds without affecting the output of the program.