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

[WIP] [ITensors] add kwarg to truncate! to return RankFactorization.Spectru… #1518

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

NuclearPowerNerd
Copy link
Contributor

@NuclearPowerNerd NuclearPowerNerd commented Jul 10, 2024

Description

The purpose of this commit is to allow the truncation error from an operation to be returned. This is the first step envisioned for that functionality.

The spectrum and truncation error from calling svd() on ITensors is not accessible from calls to truncate! and truncate. This allows the Spectrum type (which contains the spectrum and the error) to be optionally returned. This required importing RankFactorization.Spectrum in ITensors (so an added import in imports.jl) and then slightly refactoring the definition of truncate! and truncate.

This commit makes a named tuple with each element being "bond_n" where n is 1 to N-1 bonds and each element of the named tuple corresponds to the Spectrum returned from the call to svd as the bonds are swept over during the call to truncate.

All tests were run with 92979 passing and 73 broken. Total tests were 93052.

Implements #1516

If practical and applicable, please include a minimal demonstration of the previous behavior and new behavior below.

Minimal demonstration of previous behavior

truncate!(m, maxdim=10)
# or
m = truncate!(m, maxdim=10)

Minimal demonstration of new behavior

m, spec = truncate!(m, maxdim=10, return_spectrum=true)
spec.bond_1.eigs  # inspect square of singular values
spec.bond_1.truncerr # inspect truncation error

Additional Remark

I limited the scope of these changes to just truncate! and truncate. But looking forward, it would be nice to be able to get truncation error returned for any operation. For example, something like this

m, spec = contract(M, a, alg="densitymatrix", maxdim=5, return_spectrum=true)

I wasn't sure if those changes should be implemented as part of this pull request or be made in a separate pull request. So I'd appreciate feedback on what the preference there is.

How Has This Been Tested?

I verified its behavior in the MWE above. If I need to add a dedicated test let me know. I didn't see an existing one for truncate! in /ITensors.jl/src/lib/ITensorMPS/test/base/.

Checklist:

  • 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.
  • I have performed a self-review of my own code.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have added tests that verify the behavior of the changes I made.
  • 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.

ITensor#1516)

The purpose of this commit is to allow the truncation error from an
operation to be returned. This is the first step envisioned for that
functionality.

The spectrum and truncation error from calling svd() on ITensors is not
accessible from calls to truncate! and truncate. This allows the
Spectrum type (which contains the spectrum and the error) to be
optionally returned. This required importing RankFactorization.Spectrum
in ITensors (so an added import in imports.jl) and then slightly
refactoring the definition of truncate! and truncate.

This commit makes a named tuple with each element being "bond_n" where n
is `1` to `N-1` bonds and each element of the named tuple corresponds to
the Spectrum returned from the call to svd as the bonds are swept over
during the call to truncate.

All tests were run with 92979 passing and 73 broken. Total tests were
93052.
@NuclearPowerNerd
Copy link
Contributor Author

Minor comment I just realized. The documentation needs to be clarified from ...and then inspect the singular values and ... to ...and then inspect the square of the singular values and ....

@mtfishman
Copy link
Member

I don't have time to review right now, but I don't like this interface, @emstoudenmire we should discuss before merging.

@mtfishman mtfishman changed the title [ITensors] add kwarg to truncate! to return RankFactorization.Spectru… [WIP] [ITensors] add kwarg to truncate! to return RankFactorization.Spectru… Jul 10, 2024
@mtfishman mtfishman marked this pull request as draft July 10, 2024 17:40
@NuclearPowerNerd
Copy link
Contributor Author

I don't have time to review right now, but I don't like this interface, @emstoudenmire we should discuss before merging.

Thanks for the quick response Matt. When you do have time, I'd be interested in understanding why you don't like it and how I could improve it.

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.

2 participants