-
Notifications
You must be signed in to change notification settings - Fork 279
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
Daisyrainsmith/commutation2 #417
base: develop
Are you sure you want to change the base?
Daisyrainsmith/commutation2 #417
Conversation
Update to add commutation rules to the optimizer engine. 1. Update _optimizer.py and _optimizer_test.py - apply_commutation Boolean – tells LocalOptimizer whether to consider commutation rules - Rearranged existing code to streamline and improve readability. - Restructuring of optimizer to check if the next command is commutable with the current command, or if it may be part of a commutable circuit. 2. Update _basics.py and _basics_test.py - Added attributes to do with commutation rules. 3. Update _command.py and _command_test.py - Added commutation attributes and overlap function to streamline optimizer code. 4. Update _gates.py and _gates_test.py - Added attributes to do with commutation rules. - Updated Rxx, Ryy, Rzz to have interchangeable qubit indices. 5. Update _metagates.py - Added commutable rule. 6. Addition of _relative_command.py and _relative_command_test.py - Dummy command which can be used without an engine, used to define commutation rules. 7. Update restricted_gate_set.py - By default set apply_commutation = True
- Commutability enum - Commutation rules for X with Rx, Ph and SqrtX, plus tests in _optimize_test, _command_test and _gates_test
- Added more commutable gates to the _commutable_gates list for gates: X, Y, Z, Rx, Ry, Rz, Rxx, Ryy, Rzz, SqrtX, Ph, S, T, R - Added parameterized unit tests to check these commutation rules are recognized by the LocalOptimizer - Minor stylistic changes - I haven’t added tests to check that S, T and SqrtX commute through their commuting partners because as far as I know they don’t cancel/merge with themselves, so there is no way of checking/no need for that functionality.
- Commutability Enum changed to IntEnum. - Commutable circuit added to Ph and R, and commutable circuit tests in optimize_test parameterised to test R, Rz, Ph.
- Also fix some errors in the tests where Rxx, Ryy and Rzz were assumed to be single-qubit gates - Potential API break for users: Rxx(1.0) | qubit1 + qubit2 was considered valid and now should be replaced with: Rxx(1.0) | (qubit1, qubit2)
Adding commutation logic Rewrite of PR # 386
There are a lot of failing tests that you should probably fix before I will review this again. |
Hi Takishima, Could you be more specific? When I run tests on my computer, ops and cengines pass all tests. These are the only folders which I expect to be affected by the new code. There are a lot of tests that fail that I can't see any relation to my new code, such as: libs/math/_gates_math_test and backends/_aqt/_aqt_test. Could you tell me which tests fail for you that you think are related to the new code? Thanks, Daisy. |
Ok, so here is a small breakup of the CI test failures: Git issuesTry to run a diff using Git between this branch and the latest
and then maybe:
And see if any files appear, beside the ones you know you have modified. Since one of your commit mentions a manual rebase, you might have overlooked something. CI failuresStatic analysisLink: https://github.com/ProjectQ-Framework/ProjectQ/actions/runs/1445382789 Since the last major code update, ProjectQ is now completely checked using static analysis tools like:
Some of the issues can be fixed very easily on your side (mainly all the issues linked to formatting discrepancies). In order to solve those, simply do the following:
For the other checks (ie. mainly linters such as My suggestions for errors due to too many statements or too many branches ( Unit testsLink: https://github.com/ProjectQ-Framework/ProjectQ/actions/runs/1445382791 In these tests, some of the errors are indeed quite puzzling and I did not have the time to investigate them in much details over the last few days, but I'll try to dive deeper next week if I find some time. For sure, the errors about Some others are probably linked to the fact that the new optimizer is enabling commutation by default and so therefore some of the assumptions made when writing tests are not satisfied anymore. This is most likely the case of the |
for more information, see https://pre-commit.ci
|
Adding commutation logic
Rewrite of PR # 386