-
Notifications
You must be signed in to change notification settings - Fork 1.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
Apply as_sweep for SqrtCZGauge #6931
base: main
Are you sure you want to change the base?
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #6931 +/- ##
=======================================
Coverage 97.86% 97.86%
=======================================
Files 1084 1084
Lines 94315 94327 +12
=======================================
+ Hits 92301 92316 +15
+ Misses 2014 2011 -3 ☔ View full report in Codecov by Sentry. |
two_qubit_gate=ops.ISWAP, | ||
pre_q0=xy_a, | ||
pre_q1=xy_b, | ||
post_q0=xy_b, | ||
post_q1=xy_a, | ||
support_sweep=True, | ||
two_qubit_gate=ops.ISWAP, pre_q0=xy_a, pre_q1=xy_b, post_q0=xy_b, post_q1=xy_a |
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.
It looks like you committed a lot of files that you didn't intend to.
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.
That is intended.
There are 2 commits in this,
- the first commit apply as_sweep for SqrtCZGauge.
- the second commit cleanup the support_sweep parameter as all the gauges will by default support sweep.
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.
Regarding the second point, do you then need to specify somewhere that the sycamore gauge does not support sweeps yet? (https://github.com/quantumlib/Cirq/blob/main/cirq-google/cirq_google/transformers/sycamore_gauge.py)
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.
Oh, good point! as_sweep is already supported for sycamore gauge. We don't need to add extra code /comments here to support sycamore gauge.
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.
this is great but lets allow some flexiblity when parameterizing two qubit gates
def __init__( | ||
self, | ||
# target can be either a specific gate, gatefamily or gateset | ||
# which allows matching parametric gates. | ||
target: Union[ops.Gate, ops.Gateset, ops.GateFamily], | ||
gauge_selector: Callable[[np.random.Generator], Gauge], | ||
symbolize_2_qubit_gate_fn: Optional[ | ||
Callable[[ConstantGauge, sympy.Symbol], Tuple[ops.Gate, float]] |
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.
lets allow for some flexibility and make the signature deal with lists
Callable[[ConstantGauge, Sequence[sympy.Symbol]], Sequence[Tuple[ops.Gate, Union[float, int]]]]
Example usage:
exec
parameterized_circuit, sweeps = self.gauge_transformer.as_sweep(input_circuit, N=5)
with input_circuit:
output parameterized circuit is:
randomly generalized sweeps