Skip to content
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.

Sign up for GitHub

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

Jump to bottom

NoCompile Tag for optimizers #4253

Closed
dstrain115 opened this issue Jun 22, 2021 · 12 comments
Closed

NoCompile Tag for optimizers #4253

dstrain115 opened this issue Jun 22, 2021 · 12 comments
Assignees
@tanujkhattar
Labels
area/tags area/transformers kind/feature-request Describes new functionality needs agreed design We want to do this, but it needs an agreed upon design before implementation triage/needs-feasibility [feature requests] needs design work to prove feasibility before accepting

Comments

@dstrain115
Copy link
Collaborator

Is your feature request related to a use case or problem? Please describe.
Certain use cases involve creating gates that should not be optimized. For instance, if you add spin echoes to an idle qubit to decrease dephasing errors, you do not want a merging optimizers to collapse them down again. Other benchmarking cases may want this as well.

Describe the solution you'd like
I would like to add a tag (either to cirq-core or to cirq-google) that specifies that compiles should skip this gate. Perhaps NoOptimizeTag() or something like that.

[optional] Describe alternatives/workarounds you've considered

What is the urgency from your perspective for this issue? Is it blocking important work?
P1 - I need this no later than the next release (end of quarter)

High priority nice-to-have, since it is blocking a nice end-to-end example of combining various optimization techniques together in order to increase fidelity.
@dstrain115 dstrain115 added the kind/feature-request Describes new functionality label Jun 22, 2021
@vtomole
Copy link
Collaborator

vtomole commented Jun 22, 2021

I thought this was done as part of #2642. It seems like tags were implemented but not NoOptimizeTag().

@95-martin-orion 95-martin-orion added area/transformers area/tags triage/discuss Needs decision / discussion, bring these up during Cirq Cynque labels Jul 2, 2021
@pgoiporia
Copy link

Hey! I'm new to cirq and I want to create/use a NoOptimizeTag. How should I do it? More specifically, I was hoping to create a cirq equivalent of barriers in qiskit/openqasm, so would there be a way to tag a group of successive operations with a NoOptimizeTag? Thank you!

@pgoiporia pgoiporia mentioned this issue Jul 7, 2021
Closed
@pgoiporia
Copy link

Created a draft pull request for this issue. Created a nocompile tag and edited the drop_negligible optimizer so as to ignore any gate that has a nocompile tag to it. Need to create test cases for the drop_negligible. Will continue editing the other optimizers as well and create tests for them.

@tanujkhattar tanujkhattar mentioned this issue Aug 23, 2021
Merged
@MichaelBroughton MichaelBroughton added needs agreed design We want to do this, but it needs an agreed upon design before implementation and removed triage/discuss Needs decision / discussion, bring these up during Cirq Cynque labels Aug 25, 2021
@tanujkhattar
Copy link
Collaborator

This will be taken care of during the transformer patterns roadmap item. #3237

@github-actions
Copy link

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 30 days

@tanujkhattar
Copy link
Collaborator

We need to further clarify the requirements for supporting NoCompile tag in transformers. For example, Let op be an operation tagged with NoCompile tag which occurs in moment i.

  1. An obvious constraint on op is that transformers should not merge/decompose/drop the operation op and hence op should still be present in the circuit. But 2. and 3. below are more nuanced.
  2. Is it okay if op gets shifted to another moment? Or should op occur in the same moment i ? Some examples in which op can get shifted to another moment is when
    a. We stratify the circuit to align 1/2q gates.
    b. Some other operations to the left of op are merged/decomposed, hence destroying the moment structure of the circuit.
  3. Is it allowed to move other commuting operations across op, without changing op?

I would expect a NoCompile tag to satisfy only 1. and a barrier to satisfy 1. and 3..

@MichaelBroughton MichaelBroughton added the triage/needs-feasibility [feature requests] needs design work to prove feasibility before accepting label Oct 6, 2021
CirqBot pushed a commit that referenced this issue Oct 12, 2021
@tanujkhattar
Replace isinstance(op, GateOperation) checks in cirq_google optimiz…
9f3034c 
…ers to support other operation types. (#4459)

Proliferation of `isinstance(op, GateOperation)` checks results in many inconsistencies due to different available operation types like `ControlledOperations` and `TaggedOperations`. This PR fixes #4152 and is a first step towards fixing #3556 

Note that `TaggedOperations` which were earlier ignored by the optimizers would now be considered, and hence this is potentially a breaking change if people were implicitly relying on TaggedOperations not getting compiled by the optimizers. Since the optimizer doesn't document / test this behavior, I consider it to be a bug rather than a feature and an explicit `NoCompile` tag should be implemented as part of #4253 


This PR is blocked on submitting #4167 (tests will stop failing once the PR is submitted and this rebased). 

Update: This is now ready for review.
@github-actions
Copy link

github-actions bot commented Nov 6, 2021

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 30 days

@github-actions
Copy link

github-actions bot commented Dec 6, 2021

Issue closed due to inactivity.

@github-actions github-actions bot closed this as completed Dec 6, 2021
@tanujkhattar tanujkhattar reopened this Dec 6, 2021
@github-actions
Copy link

github-actions bot commented Jan 6, 2022

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 30 days

This was referenced Jan 24, 2022
Merged
Merged
@tanujkhattar tanujkhattar mentioned this issue Jan 28, 2022
Merged
CirqBot pushed a commit that referenced this issue Jan 31, 2022
@tanujkhattar
Add synchronize_terminal_measurements transformer to replace `Synch…
ab61d7f 
…ronizeTerminalMeasurements` (#4911)

- Part of #4722
- Follows the new Transformer API #4483
- Supports no compile tags NoCompile Tag for optimizers #4253
- Fixes #4907
This was referenced Feb 3, 2022
Merged
Merged
Merged
@github-actions
Copy link

github-actions bot commented Feb 6, 2022

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 30 days

This was referenced Feb 7, 2022
Merged
Closed
This was referenced Feb 12, 2022
Merged
Closed
Merged
Merged
This was referenced Feb 25, 2022
Merged
Merged
Merged
@github-actions
Copy link

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 30 days

CirqBot pushed a commit that referenced this issue Mar 14, 2022
@tanujkhattar
Add cg.SycamoreTargetGateset and deprecate `cg.ConvertToSycamoreGat…
889e5d2 
…es` (#5054)

- Part of #5028
- Follows the new Transformer API #4483
- Supports no compile tags NoCompile Tag for optimizers #4253
@tanujkhattar
Copy link
Collaborator

All circuit optimizers have been migrated to the new transformer framework, which supports adding no compile tags for every transformer. I'll therefore mark this as complete and close the issue.

rht pushed a commit to rht/Cirq that referenced this issue May 1, 2023
@tanujkhattar @rht
Replace isinstance(op, GateOperation) checks in cirq_google optimiz…
653f846 
…ers to support other operation types. (quantumlib#4459)

Proliferation of `isinstance(op, GateOperation)` checks results in many inconsistencies due to different available operation types like `ControlledOperations` and `TaggedOperations`. This PR fixes quantumlib#4152 and is a first step towards fixing quantumlib#3556 

Note that `TaggedOperations` which were earlier ignored by the optimizers would now be considered, and hence this is potentially a breaking change if people were implicitly relying on TaggedOperations not getting compiled by the optimizers. Since the optimizer doesn't document / test this behavior, I consider it to be a bug rather than a feature and an explicit `NoCompile` tag should be implemented as part of quantumlib#4253 


This PR is blocked on submitting quantumlib#4167 (tests will stop failing once the PR is submitted and this rebased). 

Update: This is now ready for review.
rht pushed a commit to rht/Cirq that referenced this issue May 1, 2023
@tanujkhattar @rht
Add synchronize_terminal_measurements transformer to replace `Synch…
03ddbd8 
…ronizeTerminalMeasurements` (quantumlib#4911)

- Part of quantumlib#4722
- Follows the new Transformer API quantumlib#4483
- Supports no compile tags NoCompile Tag for optimizers quantumlib#4253
- Fixes quantumlib#4907
rht pushed a commit to rht/Cirq that referenced this issue May 1, 2023
@tanujkhattar @rht
Add cg.SycamoreTargetGateset and deprecate `cg.ConvertToSycamoreGat…
f4d1a50 
…es` (quantumlib#5054)

- Part of quantumlib#5028
- Follows the new Transformer API quantumlib#4483
- Supports no compile tags NoCompile Tag for optimizers quantumlib#4253
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@tanujkhattar tanujkhattar

Labels
area/tags area/transformers kind/feature-request Describes new functionality needs agreed design We want to do this, but it needs an agreed upon design before implementation triage/needs-feasibility [feature requests] needs design work to prove feasibility before accepting
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[WIP] NoCompile Tag for optimizers pgoiporia/Cirq
6 participants
@MichaelBroughton @tanujkhattar @vtomole @dstrain115 @95-martin-orion @pgoiporia