ITensors.jl icon indicating copy to clipboard operation
ITensors.jl copied to clipboard

[ITensorMPS] Bond dimensions produced by randomCircuitMPS

Open markusschmitt opened this issue 3 years ago • 14 comments

Description

Fixes #869

How Has This Been Tested?

Please describe the tests that you ran to verify your changes.

  • [x] Test bond dimensions for L=10. This test is included in test/mps.jl as testset "randomCircuitMPS bond dimensions".
using ITensors
using Test

L=10
chi=128

sites = siteinds("S=1/2", L)
mps=myRandomCircuitMPS(ComplexF64,sites,chi)

expected_dims = [2, 4, 8, 16, 32, 16, 8, 4, 2]
for i in 1:9
  l = getfirst(x->hastags(x,"Link,l=$(i)"),inds(mps[i]))
  @test dim(l) == expected_dims[i]
end

Checklist:

  • [x] My code follows the style guidelines of this project. Please run using JuliaFormatter; format(".") in the base directory of the repository (~/.julia/dev/ITensors) to format your code according to our style guidelines.
  • [x] I have performed a self-review of my own code.
  • [x] I have commented my code, particularly in hard-to-understand areas.
  • [ ] I have made corresponding changes to the documentation.
  • [ ] My changes generate no new warnings.
  • [ ] Any dependent changes have been merged and published in downstream modules.

markusschmitt avatar Mar 17 '22 13:03 markusschmitt

Thanks @markusschmitt.

My major concern here would be ensuring that by truncating in this way the orthogonality center is still at site 1. Would you mind checking that?

mtfishman avatar Mar 17 '22 13:03 mtfishman

Yes, let me check that. I am actually not sure.

markusschmitt avatar Mar 17 '22 13:03 markusschmitt

If it does turn out to be an issue, a simple fix would be to determine at what site the orthogonality gets broken (say site n), set the orthogonality center range as 1:n, and then call orthogonalize!(psi, 1), so that it does the minimal orthogonalization needed. But hopefully there won't be anything to fix.

mtfishman avatar Mar 17 '22 13:03 mtfishman

@dylex do you know why the Jenkins test failed here? I've been seeing occasional errors, can we just increase the timeout time?

mtfishman avatar Mar 17 '22 13:03 mtfishman

I thought I fixed this but maybe it's still not right. I'll take a closer look. In the mean time I manually re-ran the test and now it's failing for non-timeout reasons at least.

dylex avatar Mar 17 '22 16:03 dylex

I thought I fixed this but maybe it's still not right. I'll take a closer look. In the mean time I manually re-ran the test and now it's failing for non-timeout reasons at least.

Thanks. Indeed, this PR broke a test. Is there a way that I can rerun tests (without making a new commit)? I didn't see a way from the Jenkins online interface (https://jenkins.flatironinstitute.org/blue/organizations/jenkins/ITensors.jl/detail/PR-870/2/pipeline).

mtfishman avatar Mar 17 '22 16:03 mtfishman

If you register a user on jenkins I can add you to be able to trigger builds.

dylex avatar Mar 17 '22 16:03 dylex

If you register a user on jenkins I can add you to be able to trigger builds.

That would be great, thanks. I just registered with the User ID "Matthew" and name "Matthew Fishman".

mtfishman avatar Mar 17 '22 16:03 mtfishman

Nice to see this "bounding" approach to solving the issue, Markus. I think if we can verify even theoretically that the extra truncations still preserve or guaranteed the right-orthogonality property of the MPS tensors (so that the orthogonality center remains well-defined and at site 1) then we're in business.

emstoudenmire avatar Mar 17 '22 18:03 emstoudenmire

I think the right-orthogonality may be guaranteed to remain true for any value of chi, including the new values being determined by the code in this PR. The reason is that chi is fed into the function _rmatrix, which makes a unitary (or orthogonal if real) matrix of the requested dimensions. So as long as the column dimensions of these matrices are greater than or equal to the row dimensions (in this case, as long as chi <= dim(sites[j]) * dim(l[j])) then the resulting tensors will be right-orthogonal. We could add an assertion that checks the above inequality always holds to ensure orthogonality.

emstoudenmire avatar Mar 17 '22 18:03 emstoudenmire

@emstoudenmire: I think the condition chi <= dim(sites[j]) * dim(l[j]) is already ensured in the current version when calculating the maximal bond dimensions.

I also checked the orthogonality as follows:

using Test
using ITensors

L=6
chi=128

sites = siteinds("S=1/2", L)

mps=randomCircuitMPS(ComplexF64,sites,chi)

right_contracted = mps[L] * conj(prime(mps[L], tags="Link"))
@test isapprox(norm(right_contracted-delta(inds(right_contracted))), 0.0, atol=1e-14)
for j in (L - 1):-1:2
    right_contracted = (mps[j] * right_contracted) * conj(prime(mps[j], tags="Link"))
    @test isapprox(norm(right_contracted-delta(inds(right_contracted))), 0.0, atol=1e-14)
end

This could be added as a test, possibly with different smaller chi to check for the cases mentioned by Miles.

markusschmitt avatar Mar 21 '22 10:03 markusschmitt

Great – I just checked and we do already have a similar test in our test/mps.jl file, so no need to add another one, but it's good you did that check yourself. The new test you added is good to have to ensure the bond dimensions now have the minimal values.

The current test isn't passing, though, because randomCircuitMPS is not exported by the ITensors module, as it's an internal function only. Please update the test to use the randomMPS interface instead so that the tests can pass.

Also it looks like you'll need to make the update pointed out by Matt, about changing the name myRandomCircuitMPS back to randomCircuitMPS in src/mps/mps.jl.

emstoudenmire avatar Mar 21 '22 15:03 emstoudenmire

Sorry, I am only getting back to this now. With the new commits the tests should pass. I had to slightly change another test as well.

markusschmitt avatar Jul 13 '22 11:07 markusschmitt

Codecov Report

Merging #870 (ec8c9a8) into main (6a83a83) will increase coverage by 0.02%. The diff coverage is 100.00%.

:exclamation: Current head ec8c9a8 differs from pull request most recent head 238e4dc. Consider uploading reports for the commit 238e4dc to get more accurate results

@@            Coverage Diff             @@
##             main     #870      +/-   ##
==========================================
+ Coverage   67.12%   67.14%   +0.02%     
==========================================
  Files          88       88              
  Lines        9389     9395       +6     
==========================================
+ Hits         6302     6308       +6     
  Misses       3087     3087              
Impacted Files Coverage Δ
src/mps/mps.jl 91.91% <100.00%> (+0.12%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 6a83a83...238e4dc. Read the comment docs.

codecov-commenter avatar Jul 13 '22 12:07 codecov-commenter