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.

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

Speed up simplification of CNOTs in BooleanHamiltonian #4532

Closed
tonybruguier opened this issue Sep 25, 2021 · 4 comments
Closed

Speed up simplification of CNOTs in BooleanHamiltonian #4532

tonybruguier opened this issue Sep 25, 2021 · 4 comments
Labels
area/performance kind/health For CI/testing/release process/refactoring/technical debt items

Comments

@tonybruguier
Copy link
Contributor

Description of the issue
During the review of #4386 which reduced the number of CNOT gates produced by BooleanHamiltonian, there was a remark that the functions _simplify_cnots_triplets and _simplify_commuting_cnots can be made faster. As soon as they find a simplification, they return early and then start from scratch again. Maybe continuing would be more efficient.

Cirq version
0.13.0.dev

@tonybruguier tonybruguier added the kind/health For CI/testing/release process/refactoring/technical debt items label Sep 25, 2021
@yjt98765
Copy link
Contributor

yjt98765 commented Nov 4, 2021

@tonybruguier I have some initial ideas on optimizing the _simplify_commuting_cnots, but I am not sure about the constraints, more specifically, a test case:

        # Can simplify, but violates CNOT ordering assumption
        ([(0, 1), (2, 3), (0, 1)], False, False, [(0, 1), (2, 3), (0, 1)]),

I know that the current algorithm does not simplify this test case, but is it allowed to be simplified? What ordering assumption does it violate? Here says The code does not make any assumption as to the order of the CNOTs. I have also checked the paper but still have no clue. Is _simplify_cnots_triplets less strict as to the order of CNOTs?

@tonybruguier
Copy link
Contributor Author

So the goal of the issue is simply to speed up the for loops.

The way the code works is to repeatedly be called. If a simplification is found, we apply it and repeat from scratch. If no simplification is found, then we exit.

Instead of restarting from scratch, we could do something smarter. It's not really a quantum problem, but rather a classical algorithmic optimization.

Of course, there could be other optimizations, but that's what the intent of this issue was.

@yjt98765
Copy link
Contributor

yjt98765 commented Nov 4, 2021

Thank you for the quick response. I agree that this issue is not about the quantum part. Can I create a PR only to optimize _simplify_commuting_cnots? I think the PR will produce the same result as the current code, with potentially fewer loops. _simplify_cnots_triplets is somewhat more complex and I am not sure how to improve.

@tonybruguier
Copy link
Contributor Author

Sounds good. One function at a time seems like a good idea. I could do an initial review, but the final approval would have to be given be an expert.

CirqBot pushed a commit that referenced this issue Sep 9, 2022
Clean up TODO-s for fixed issues #4328 and #4532
rht pushed a commit to rht/Cirq that referenced this issue May 1, 2023
harry-phasecraft pushed a commit to PhaseCraft/Cirq that referenced this issue Oct 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/performance kind/health For CI/testing/release process/refactoring/technical debt items
Projects
None yet
Development

No branches or pull requests

3 participants