-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Adds a simple scheduler that just follows the moment structure. #78
Conversation
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 few minor nits. Otherwise LGTM.
cirq/schedules/schedulers.py
Outdated
device.validate_scheduled_operation(schedule, scheduled_op) | ||
# Increment time for next moment by max of ops during this moment. | ||
max_duration = max((device.duration_of(op) for op in moment.operations), | ||
key=Duration.total_picos) |
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.
Duration implements comparison operations, so this key argument should be unnecessary.
t += max(device.duration_of(op) for op in moment.operations)
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.
Funny I tried that and if failed but now it works. ¯_(ツ)_/¯
cirq/schedules/schedulers.py
Outdated
schedule = Schedule(device) | ||
t = Timestamp() | ||
for moment in circuit.moments: | ||
if len(moment.operations) == 0: |
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.
if not moment.operations:
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.
Done
cirq/schedules/schedulers_test.py
Outdated
schedule = schedulers.moment_schedule(device, circuit) | ||
|
||
zero_ns = Timestamp() | ||
second = Timestamp(nanos=20) |
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.
Maybe call it a tick instead of a "second".
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.
ha that is "second" as in "2nd" not the time. Changed to twenty_ns
cirq/schedules/schedulers.py
Outdated
from cirq.time import Timestamp | ||
|
||
|
||
def moment_schedule(device: Device, circuit: Circuit): |
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.
Name is okay, but not great. Maybe moment_by_moment_schedule
?
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.
Done
I think this will be useful at is a very simple conceptual scheduler.