Skip to content

Conversation

@verult
Copy link
Collaborator

@verult verult commented May 31, 2022

Per @tanujkhattar 's suggestion, it would be useful to have the most general GateFamily for a particular gate instance as the key of GridDeviceMetadata.gate_durations (i.e. cg.FSimGateFamily(gates_to_accept=[cg.SYC]) instead of cirq.GateFamily(cg.SYC)). However, constructing the GateFamily to look up a particular gate duration is additional overhead for the user.

Here I propose a GateDurationTable data structure which is initialized using the most general GateFamilies for each device gate, while allowing users to look up duration using specific gate instances. For example:

>>> gdt = GateDurationTable({ cg.FSimGateFamily(gates_to_accept=[cg.SYC]): cirq.Duration(nanos=1) })

>>> gdt[cg.SYC]
cirq.Duration(nanos=1)

>>> gdt[cirq.FSimGate(math.pi/2, math.pi/6)]
cirq.Duration(nanos=1)

Users can still see the full durations list via print(gdt).

How does this look conceptually?

We may not want to overcomplicate gate duration logic since it's mostly informational and values are approximate, but I think this is a low hanging fruit where we can make the experience better.

@MichaelBroughton @tanujkhattar

@MichaelBroughton
Copy link
Collaborator

MichaelBroughton commented Jun 1, 2022

Hmmm I'm on the fence for this one. It seems like if I'm a user who wants to explore duration related concepts I'd probably do something like:

my_op = <some operation in my circuit>
durations = my_device.metadata.gate_durations
op_duration = 'unknown'
for family in durations:
  if my_op in family:
    op_duration = durations[family]

# ... do stuff with op_duration ...

Do we need a whole new datatype for this kind of lookup just for durations ?

Maybe we could do something a little more general like a GateDict that behaves like a GateSet but can do similar Key, Value mappings with a similar types -> gates -> operations containment check functionality. Then if we wanted we could return a GateDict. What do people think ?

@verult
Copy link
Collaborator Author

verult commented Jun 1, 2022

Right, seems like documenting the expected duration lookup procedure using the existing dictionary setup is better than implementing/maintaining a new data structure since duration access logic is simple enough. Closing this and I'll open a separate PR for a docstring update.

@tanujkhattar
Copy link
Collaborator

Maybe we could do something a little more general like a GateDict that behaves like a GateSet but can do similar Key, Value mappings with a similar types -> gates -> operations containment check functionality. Then if we wanted we could return a GateDict

I like this idea of extending the cirq.Gateset class itself to return a gateset.gate_family_for(gate) which will return the gate family which matches gate/operation, if they are part of the gateset. Then, a user could just do:

gate_durations[gf.gate_family_for(op)] to get duration of op.

@verult
Copy link
Collaborator Author

verult commented Jun 3, 2022

That seems easy enough to do, and also avoids the additional work of an extra data structure (maintaining serialization etc). On the other hand, I do like the feature of the separate GateDict approach that users can check the duration of a particular gate without having to worry about the implementation detail that durations are keyed by GateFamilies, although that still gets exposed when a user prints the GateDict.

No strong preference, happy to implement any approach. Hopefully some users will get to test this out before Cirq 1.0 and we could revise our design if we get feedback on this.

CirqBot pushed a commit that referenced this pull request Jun 3, 2022
rht pushed a commit to rht/Cirq that referenced this pull request May 1, 2023
harry-phasecraft pushed a commit to PhaseCraft/Cirq that referenced this pull request Oct 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size: M 50< lines changed <250

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants