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

Added Efficiency Improvements in the code. Including fast versions of… #94

Merged
merged 4 commits into from
Jun 29, 2023

Conversation

JoeyT1994
Copy link
Contributor

This update adds efficiency improvements to the apply and gauge functions by minimising the amount of graph analysis that is done.

Additionally:

  1. The apply function for a tn in the Vidal gauge places the site-index on the QR decomp relative to whether a gate is being applied.
  2. A fast version of dag(tn::AbstractITensorNetwork) has been added
  3. A function unflattened_norm_network(tn::AbstractITensorNetwork) has been added which is faster than using
  4. An example has been added to demonstrate different gauging routines

… norm and dagger. Added an example on gauging ITNS
src/apply.jl Outdated Show resolved Hide resolved
@JoeyT1994
Copy link
Contributor Author

JoeyT1994 commented Jun 28, 2023

Not really sure what is going on in test_apply here (which is why the tests are failing?). The changes to
function ITensors.apply( o::Union{ITensor,NamedEdge}, ψ::AbstractITensorNetwork, bond_tensors::DataGraph; normalize=false, apply_kwargs..., ) appear to have made the apply function extremely accurate for the simple update case and hence its accuracy is far higher than the belief propagation version (whereas they were identical before - as we expect them to be).

There must be a bug there as they should probably not be that accurate..

@mtfishman
Copy link
Member

What if you switch back from qr(...) to factorize(...; cutoff=1e-16), do the tests pass? Otherwise it seems like the rest of the changes are either independent of those tests or just code reoarganization.

@mtfishman
Copy link
Member

mtfishman commented Jun 28, 2023

I'm confused by why you are saying SU gauging is now more accurate, it looks less accurate to me with the latest commit. EDIT: Never mind, I don't understand that test.

src/gauging.jl Outdated Show resolved Hide resolved
@JoeyT1994
Copy link
Contributor Author

Okay those bugs should be fixed now and all tests should pass.

src/apply.jl Outdated Show resolved Hide resolved
@codecov-commenter
Copy link

codecov-commenter commented Jun 28, 2023

Codecov Report

Merging #94 (82b4909) into main (34fd5dc) will increase coverage by 0.10%.
The diff coverage is 90.90%.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

@@            Coverage Diff             @@
##             main      #94      +/-   ##
==========================================
+ Coverage   79.25%   79.36%   +0.10%     
==========================================
  Files          61       61              
  Lines        3442     3465      +23     
==========================================
+ Hits         2728     2750      +22     
- Misses        714      715       +1     
Impacted Files Coverage Δ
src/apply.jl 89.26% <73.68%> (-1.96%) ⬇️
src/abstractitensornetwork.jl 82.66% <100.00%> (+0.77%) ⬆️
src/gauging.jl 100.00% <100.00%> (ø)

... and 4 files with indirect coverage changes

@mtfishman
Copy link
Member

Thanks!

@mtfishman mtfishman merged commit 204f45b into ITensor:main Jun 29, 2023
@JoeyT1994 JoeyT1994 deleted the EfficiencyChanges branch June 29, 2023 15:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants