-
Notifications
You must be signed in to change notification settings - Fork 284
Daisyrainsmith/commutation2 #417
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
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 issues
Try to run a diff using Git between this branch and the latest develop:
git diff -bw --diff-algorithm=patience develop..HEAD --stat
and then maybe:
git diff -bw --diff-algorithm=patience develop..HEAD
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 failures
Static analysis
Link: 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:
clang-tidyblackisortflake8pylint
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:
-
Install
pre-commit(https://pre-commit.com/) -
Run pre-commit from within the root directory of your ProjectQ installation:
pre-commit run --all-files --hook-stage manual -
Commit the changes
For the other checks (ie. mainly linters such as flake8 and pylint) you will need to address the issues one by one. The error messages here should be pretty explanatory.
My suggestions for errors due to too many statements or too many branches (pylint), try to remove them by restructuring the code if you are close to the limit, but. if it is not feasible or not reasonable, you may add a comment to tell the tool to ignore the warning for the particular case.
Unit tests
Link: 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 IndexErrors located in _command.py at line 222 looks very suspicious and you may ignore them for now.
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 test_chooser_Ry_reducer test.
for more information, see https://pre-commit.ci
CLAassistant
commented
Oct 31, 2022
CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.
✅ Takishima
❌ daisyrainsmith
You have signed the CLA already but the status is still pending? Let us recheck it.
Adding commutation logic
Rewrite of PR # 386