Skip to content

[cirqflow] Add target_gateset to QuantumRuntimeConfiguration #5336

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

Merged
merged 6 commits into from
May 10, 2022

Conversation

mpharrigan
Copy link
Collaborator

Optionally run gate compilation on executables before running them.

I took the opportunity to flesh out the tests for QuantumRuntimeConfiguration as well. It's learning more features.

@mpharrigan mpharrigan requested review from wcourtney, a team, vtomole, cduck and verult as code owners May 7, 2022 00:07
@CirqBot CirqBot added the size: M 50< lines changed <250 label May 7, 2022
@mpharrigan
Copy link
Collaborator Author

I should say I found the default behavior of cirq.optimize_for_target_gateset when passed None as the target gateset surprising. I think it defers to using each gate's preferred target gateset via decompose() rather than doing nothing or throwing an error.

The behavior here is different: If target_gateset is None, it will not monkey with your circuit.

@mpharrigan
Copy link
Collaborator Author

Otherwise, I was absolutely delighted how easy it was to plug in the wonderfully designed transformers api!

Comment on lines +305 to +307
circuit = cirq.optimize_for_target_gateset(
circuit, gateset=rt_config.target_gateset
).freeze()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice if we can also allow plumbing a transformer context here, which can be used to control many useful compilation behaviors like context.tags_to_ignore or context.deep (to compile sub-circuits recursively instead of treating them as standalone operations).

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That sounds reasonable.

Now that I'm looking at TransformerContext it would be really nice if I could offer a transformer logger that logs into the cg.RuntimeInfo for each executable.

Designing the API to enable this may be weird: because the TransformerLogger in the TransformerContext will need a reference to the RuntimeInfo which doesn't exist when the user is setting up their QuantumRuntimeConfiguration.

I could do something gross like define a subclass WorkflowTransformerLogger and do an isinstance check during the execution loop. If it's an instance of this special class, call a method to plumb in the RuntimeInfo object.

Any other ideas?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TransformerContext isn't json serializable, which blocks its use in QuantumRuntimeConfiguration. We can plumb it through when that's done

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, it's a dataclass but not a json_serializable_dataclass. Okay, let's open a new issue to track the change.

Also; for google devices (i.e. devices with GridDeviceMetadata), it should be possible to get the target gateset from processor_record.device.metadata.compilation_target_gatesets[0] -- but I think it's better to explicitly require a target_gateset here and the user can pick it from the device instead of creating a new instance if they want to.

@tanujkhattar
Copy link
Collaborator

I should say I found the default behavior of cirq.optimize_for_target_gateset when passed None as the target gateset surprising. I think it defers to using each gate's preferred target gateset via decompose() rather than doing nothing or throwing an error.

Just FYI,

By default, if specified as None, the method defaults to using cirq.decompose on all operations, which should decompose (most of) them to Cirq's [default target gateset] (

DECOMPOSE_TARGET_GATESET = ops.Gateset(
). We also check that all cirq.decompose(op) ends at the default gateset as part of the consistent protocols test suite

But in hindsight, I agree that a good alternative could also be to give a sensible default (like CZTargetGateset) to the gateset parameter of optimize_for_target_gateset method and then either ignore or raise an error if gateset is None. Maybe we can still do this change before Cirq 1.0.

@mpharrigan
Copy link
Collaborator Author

default gateset as part of the consistent protocols test suite

I must have missed the part that we actually have a gateset we target for decompose. That makes things much nicer!

I think for the cg.workflow tools we need a way to turn it off. I don't think the different None behavior between the QuantumRuntimeConfiguration object and optimize_for_target_gateset object will be too confusing, but worth pointing out.

@tanujkhattar tanujkhattar self-assigned this May 9, 2022
@mpharrigan mpharrigan added the automerge Tells CirqBot to sync and merge this PR. (If it's running.) label May 10, 2022
@CirqBot CirqBot added the front_of_queue_automerge CirqBot uses this label to indicate (and remember) what's being merged next. label May 10, 2022
@CirqBot CirqBot merged commit 061e31f into quantumlib:master May 10, 2022
@CirqBot CirqBot removed automerge Tells CirqBot to sync and merge this PR. (If it's running.) front_of_queue_automerge CirqBot uses this label to indicate (and remember) what's being merged next. labels May 10, 2022
rht pushed a commit to rht/Cirq that referenced this pull request May 1, 2023
…umlib#5336)

Optionally run gate compilation on executables before running them. 

I took the opportunity to flesh out the tests for `QuantumRuntimeConfiguration` as well. It's learning more features.
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.

3 participants