[ITensors] HDF5.jl package extension

[emstoudenmire]

Moved HDF5 support into a package extension. Updated docs accordingly.

Also fixed a couple of unrelated build issues in the docs and an ITensorMPS module import/overload issue.

[emstoudenmire]

I'm seeing some warnings in the tests about loading the various package extensions, e.g.

 ITensors → ITensorsVectorInterfaceExt
│  ┌ Warning: Module ITensorsVectorInterfaceExt with build ID ffffffff-ffff-ffff-0000-00274c0366aa is missing from the cache.
...

I found this open Pkg.jl issue which shows that others have been seeing it, even in Julia 1.11.
JuliaLang/Pkg.jl#3557
It was supposed to be fixed in this PR JuliaLang/julia#52841 but some are still reporting it.

[emstoudenmire]

(Also sneaking in a small orthogonal update: added directsum to the docs as we discussed.)

[mtfishman]

I'm seeing some warnings in the tests about loading the various package extensions, e.g.

 ITensors → ITensorsVectorInterfaceExt
│  ┌ Warning: Module ITensorsVectorInterfaceExt with build ID ffffffff-ffff-ffff-0000-00274c0366aa is missing from the cache.
...

I found this open Pkg.jl issue which shows that others have been seeing it, even in Julia 1.11. JuliaLang/Pkg.jl#3557 It was supposed to be fixed in this PR JuliaLang/julia#52841 but some are still reporting it.

I see, thanks for investigating. That issue seems to be related to having a system image for a package already built which triggers strange behaviors when loading package extensions relying on those pre-built packages, if I'm following the discussions in those links. That makes me think it is somewhat related to the issue in #1381, i.e. that the tests aren't being run from a clean slate but rather reusing something from a previous CI run (in this case pre-built system images, in #1381 a badly-configured Manifest.toml).

For both this issue and the issue in #1381, I wonder if running the CI using a temporary environment with Pkg.activate(temp=true) as @kmp5VT is trying out in #1365 might help.

[mtfishman]

We should bump the minor version of ITensors.jl to v0.4 since technically this is a breaking change.

[emstoudenmire]

Interesting about how that warning might be related. That's definitely on my list of things to work on next (probably on the other Observers PR) i.e. try what Karl is doing in #1365 to see if that helps the Observers extension go through, and fix that warning.

[emstoudenmire]

I am trying to think of clean-ish ways to fix the issue with dag! for AbstractMPS. (Also now there's a similar issue popping up for sim!, which involves another macro with even more lines of code – not sure why tests didn't catch this before as I don't see why this PR would do anything to cause this issue. Maybe it's just one of those things.)

The "best" idea I had was just to temporarily define dag! and sim! in ITensors. Then later we can deprecate all such functions including these. But it does seem strange to introduce more code that is about to become deprecated.

An alternative is simply to duplicate the code in abstractmps.jl but in the case of sim! it's something like 30 lines of code including docstrings and things. So a lot of duplication. (When I say duplicate, I mean only for the cases not in ITensors. For most of the cases the macro does still work if I just prepend ITensors..)

The only other idea would be to somehow rewrite the macro to more explicitly list the different cases. So like

for fname in
    (:(ITensors.prime), :(ITensors.prime!), :(ITensors.setprime), :(ITensors.setprime!) ...

or something along those lines. The tricky part is how the macro cleverly rewrites the symbols to append ! and put those into the map! function so it's not a straightforward rewrite.

One could just replace the macros by two, one for the non-modifying and the other for modifying. That option would duplicate the shared docstrings, but other than that it might not be too bad. It would not duplicate much actual code, plus if we are going to deprecate the modifying versions we can later delete the second macro in that plan.

[mtfishman]

Can't the duplicated docstring just be refactored into a function that generates the docstring that can be used by both versions?

[mtfishman]

I do not think we should define dag!, etc. in ITensors.jl.

[mtfishman]

Another suggestion would be to rewrite the for loop like this:

for (fname, fname!) in (
  (:(ITensors.dag), :(dag!)),
  (:(ITensors.prime), :(ITensors.prime!)),
...
)

[emstoudenmire]

The docstring funciton is a good idea – I had not thought of that.

So then with that improvement, a pretty simple fix would just be to duplicate the code in abstractmps.jl which is some but not a lot of duplication (maybe 10-ish lines of mostly one-liner functions). And then anyway some of that code will get deprecated later.

[mtfishman]

I think I prefer this suggestion better:

for (fname, fname!) in (
  (:(ITensors.dag), :(dag!)),
  (:(ITensors.prime), :(ITensors.prime!)),
...
)

[emstoudenmire]

Yes that's definitely the best idea. Very clean. I'm going to modify it to have a third entry for the docstring and otherwise will use that pattern.

[mtfishman]

I believe to get tests passing we will need to append 0.4 to the compat entries of the downstream packages like ITensorGaussianMPS, etc.

[emstoudenmire]

Ah thanks, was just wondering the best course of action.

By the way, I wasn't able to reproduce the sim! issue again locally. So maybe it was a false alarm or one-off local issue that had to do with some temporary state my REPL was in. I'll see if the tests pass without it because they are passing again locally without that change.

[emstoudenmire]

This one should be ready to go, unless there are possibly other tests I didn't think to run?

[mtfishman]

Could you also bump the patch versions of ITensorGaussianMPS and ITensorVisualizationBase?

[emstoudenmire]

Sure thing, done. I put [no ci] at the end of the commit for that, but I can also rerun the tests if that's best.

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 79.32%. Comparing base (e8197ce) to head (4e8f7f2).
Report is 7 commits behind head on main.

❗ Current head 4e8f7f2 differs from pull request most recent head cd1f66d. Consider uploading reports for the commit cd1f66d to get more accurate results

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1382      +/-   ##
==========================================
- Coverage   79.59%   79.32%   -0.28%     
==========================================
  Files         114      114              
  Lines        9032     8875     -157     
==========================================
- Hits         7189     7040     -149     
+ Misses       1843     1835       -8     

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

[mtfishman]

Could you also add a NEWS item to the README about the 0.4 release?

[emstoudenmire]

Sure thing, I added the NEWS entry

[mtfishman]

Sure thing, I added the NEWS entry

Sorry, I meant add an entry about it here: https://github.com/ITensor/ITensors.jl/blob/main/docs/src/index.md#news which will show up on both the README.md and in the documentation.

I haven't been updating the NEWS.md file, I found it was too much work to keep it updated and it doesn't add much beyond the commit history or the documentation. Probably we should just get rid of it to avoid confusion for developers and users.

[emstoudenmire]

I updated the docs index file – please take a look. We could add other major changes since 0.3 such as e.g. replacing the original OpSum with Ops.OpSum and other big changes like that.

I went ahead and removed NEWS.md. Let me know if you want me to put it back.