-
Notifications
You must be signed in to change notification settings - Fork 118
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
[ITensors] Fix rrule
for applying quantum circuit to non-hermitian MPO with apply_dag=true
#1344
Conversation
…ring the MPO to be hermitian. Related Issue: ITensor#1314
[test ITensors mps] |
rrule
for applying quantum circuit to non-hermitian MPO with apply_dag=true
Run ITensors mps tests from comment trigger: failed ❌ |
1 similar comment
Run ITensors mps tests from comment trigger: failed ❌ |
I've already made a test for this case here: https://github.com/ITensor/ITensors.jl/blob/v0.3.57/test/ITensorLegacyMPS/ITensorChainRules/test_chainrules.jl#L172-L181, where I marked that it should throw an error but we can repurpose it now that it is working. |
Ah ok, was not aware of that test. Thanks! |
Hey, I was wondering if the PR will be included in the near future. Should I update the test in my fork and update the merge request? |
Yes, please update the test to test out this new functionality. |
[test ITensorMPS][test ITensorGaussianMPS] |
Run ITensorGaussianMPS tests from comment trigger: succeeded ✅ |
1 similar comment
Run ITensorGaussianMPS tests from comment trigger: succeeded ✅ |
Run ITensorMPS tests from comment trigger: succeeded ✅ |
1 similar comment
Run ITensorMPS tests from comment trigger: succeeded ✅ |
Looks good, thanks! |
Description
This Pull request fixes the
rrule
for applying a quantum circuit to an MPO which is not hermitian with the keywordapply_dag = true
.Instead of assuming the MPO to be hermitian, I added an if statement in Line 122 of
src/src/ITensorChainRules/mps/abstractmps.jl
, checking if the MPO is hermitian. In case the MPO is hermitian, the old behavior is applied, in casethe MPO is not hermitian, I perform the correct pullback, which involves two contributions, one from the normal application of the gate, and one coming from the daggered application of the gate.
Fixes #1314
For demonstration, I used a (modified) version of the code snipped provided in the Issue. In the beginning one can choose using Complex or Floats, QN conserved states or dense states. In all cases the output is the expected one.
Code snipped
Minimal demonstration of previous behavior
The output before the fix was:
with the last test failing, indicating the two pullbacks to be different.
Minimal demonstration of new behavior
After the fix one gets the new output:
With all test returning the desired result.
How Has This Been Tested?
I tested the behavior using the snipped in the above section. I especially run the script with all possible combinations,
I.e. the field being Floats or Complex, hermitian or not hermitian and using QN numbers or not. All tests were successful.
I did not implemented the code snipped as a test in the test folder, but it could be done if wanted.
Checklist:
using JuliaFormatter; format(".")
in the base directory of the repository (~/.julia/dev/ITensors
) to format your code according to our style guidelines.