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

[ITensors] Add docstrings for tr and update ITensor example #1186

Closed
wants to merge 2 commits into from

Conversation

emstoudenmire
Copy link
Collaborator

This PR does the following:

  • add doc string for tr(::ITensor)
  • add doc string for tr(::MPO)
  • update ITensor example about tracing to show two cases: one where the indices are the 'same' (use the tr function) and another more general case where one can use a delta tensor.

Do we want to allow users to provide Index objects to the tr function instead of using a delta tensor? We could keep things as they are for now and consider this in the future.

@codecov-commenter
Copy link

codecov-commenter commented Sep 4, 2023

Codecov Report

Patch coverage: 95.55% and project coverage change: -18.01% ⚠️

Comparison is base (8b82ad0) 85.38% compared to head (b1d8cc0) 67.38%.
Report is 5 commits behind head on main.

❗ Current head b1d8cc0 differs from pull request most recent head 80ada9c. Consider uploading reports for the commit 80ada9c to get more accurate results

❗ 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.

Additional details and impacted files
@@             Coverage Diff             @@
##             main    #1186       +/-   ##
===========================================
- Coverage   85.38%   67.38%   -18.01%     
===========================================
  Files          88       87        -1     
  Lines        8405     8385       -20     
===========================================
- Hits         7177     5650     -1527     
- Misses       1228     2735     +1507     
Files Changed Coverage Δ
src/mps/mpo.jl 24.44% <ø> (-72.13%) ⬇️
src/tensor_operations/matrix_algebra.jl 95.34% <ø> (ø)
src/qn/flux.jl 91.66% <81.81%> (-8.34%) ⬇️
src/global_variables.jl 100.00% <100.00%> (ø)
src/qn/qnitensor.jl 69.54% <100.00%> (-14.80%) ⬇️
src/tensor_operations/tensor_algebra.jl 87.27% <100.00%> (-2.59%) ⬇️

... and 33 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

src/mps/mpo.jl Outdated
@@ -531,6 +531,15 @@ function logdot(M1::MPO, M2::MPO; make_inds_match::Bool=false, kwargs...)
return _log_or_not_dot(M1, M2, true; make_inds_match=make_inds_match)
end

"""
tr(M::MPO, kwargs...)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
tr(M::MPO, kwargs...)
tr(M::MPO; kwargs...)

# the prime levels and tags. Indices that are not in pairs
# are not traced over, corresponding to a "batched" trace.
"""
tr(T::ITensor, kwargs...)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
tr(T::ITensor, kwargs...)
tr(T::ITensor; kwargs...)

@mtfishman
Copy link
Member

Thanks. Yeah, the idea was to allow specifying more general pairs of indices to trace over, say with a syntax:

tr(A, i => j, k => l)

but we could leave that for a future PR. I think I held off on it since I didn't come across a particular need for it yet and therefore wanted to wait to see if an application would help guide what the functionality and interface might be.

I also believe I didn't document this function since I wasn't so sure about the plev and tags keyword argument interface, and also wasn't sure how those would interplay with passing indices explicitly.

For example, I could imagine a more general interface like:

tr(A; mapinds=(i -> i'))
tr(A; mapinds=(i -> replacetags(i, "a" => "b")))

as a more general version of plev and tags, where it finds pairs of indices that satisfy i == mapinds(i).

Explicitly specifying pairs of indices could even be handled through that common interface, for example:

tr(A; mapinds=(x -> Dict(i => j, k => l)[x]))

though of course we could have the convenient shorthand tr(A, i => j, k => l).

I think also it may be nice to think about how a tr function may be defined more generally for ITensorNetwork objects as a way to guide how the ITensor version of tr is defined. An example of that is I want to develop an ITensorNetworkMap type which is an ITensorNetwork with additional information that it is a map from one set of site/external indices of the tensor network to another. Then, tr(::ITensorNetworkMap) would be fully well defined since it would trace over those sets of indices. However, I hadn't fully thought through how to generally but also conveniently define an ITensorNetworkMap. The mapinds concept/interface above may be the best way to go, so it would be nice if the tr function had a consistent interface with that. That would make the code and interface simpler and more consistent, since then for example we could have simple definitions like:

tr(A::ITensorNetwork; mapinds=(i -> i')) = tr(ITensorNetworkMap(tn; mapinds))

The ITensorNetworkMap would be more generally useful, since it could be used to define networks that can get passed to dmrg or tdvp that don't follow the standard prime level convention that is currently assumed by those functions.

I think however those more general issues play out, the syntax tr(A) would still be used to trace over pairs of indices with prime levels 0 or 1 (just like functions like dmrg or tdvp would still by default assume that prime level convention). What do you think of leaving off documenting plev and tags for now while we think through a more general interface?

@emstoudenmire
Copy link
Collaborator Author

Interesting. I really like the mapinds argument idea, which is a very nice and compact use of the lambda function syntax. It both looks good, can be used inside of the function to actually map indices, and gives a clearer meaning about what the arrow direction indicates, versus in the Pair notation (0 => 1) where the purpose of an arrow always felt a little unclear to me.

No problem to leave off the keyword args for now. We could tell users to simply prepare their tensors to have the same tags and the 0=>1 prime level patter for now.

One very off-hand thought is that it might be interesting to unify the concept of an ITensorNetworkMap with the idea of a unitary ITensor, which would also "know" which are its two sets of indices. But that might be a bit of overkill as well.

@mtfishman
Copy link
Member

Another complication is that I'm not sure if the current implementation of tr is the best one. For many indices and QN blocks, I've found that using combiners in that way (combining many indices into one big index) can be fairly slow. I experimented with some alternatives a bit in this ED code: https://github.com/ITensor/ITensors.jl/blob/20aae8ce288c3cb4c3b8cd4f5276a6831752fcc9/examples/exact_diagonalization/exact_diagonalization.jl where I tested out combining pairs of indices at a time in a "fusion tree" pattern, though for tracing it may be better to trace in pairs of indices at a time. Anyway, that may just be a different issue.

I agree that the ITensorNetworkMap is conceptually related to a Unitary ITensor, and also an Identity tensor which would be an identity between pairs of indices (say with an interface I((i, j, k), (l, m, n)) or I(i => l, j => m, k => n) which would be an identity from the indices (i, j, k) to (l, m, n)). Though in terms of code design those would likely be designed at a lower level than the ITensorNetworkMap, since those would be at the tensor storage level in NDTensors. So I think in practice there will be different types of map-like ITensor and ITensorNetwork objects.

@@ -531,6 +531,11 @@ function logdot(M1::MPO, M2::MPO; make_inds_match::Bool=false, kwargs...)
return _log_or_not_dot(M1, M2, true; make_inds_match=make_inds_match)
end

"""
tr(M::MPO; kwargs...)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
tr(M::MPO; kwargs...)
tr(M::MPO)

"""
tr(M::MPO; kwargs...)

Compute the trace of the MPO `M`.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe cross reference tr(T::ITensor) for which indices it traces over?

# the prime levels and tags. Indices that are not in pairs
# are not traced over, corresponding to a "batched" trace.
"""
tr(T::ITensor; kwargs...)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
tr(T::ITensor; kwargs...)
tr(T::ITensor)

Comment on lines +19 to +20
Trace an ITensor over pairs of indices determined by
the optional prime levels and tags passed as keyword arguments.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should reword to say it is over pairs of indices with prime levels 0 and 1.

@mtfishman mtfishman changed the title Add docstrings for tr and update ITensor example [ITensors] Add docstrings for tr and update ITensor example Oct 31, 2023
@mtfishman mtfishman closed this Apr 28, 2024
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