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] [BUG] Regression introduced to ITensors during refactor of NDTensors away from kwargs... #1297

Closed
brian-dellabetta opened this issue Jan 2, 2024 · 2 comments · Fixed by #1300
Labels
bug Something isn't working ITensors Issues or pull requests related to the `ITensors` package.

Comments

@brian-dellabetta
Copy link
Contributor

brian-dellabetta commented Jan 2, 2024

Description of bug

A recent change to NDTensors introduced a regression into ITensors' replacebond! function for mps, see here for diff and responsible call to factorize just underneath. Previously, users could set min_blockdim, use_absolute_cutoff, and use_relative_cutoff in the keyword args passed to replacebond!, and kwargs... would be passed correctly to factorize. This refactor away from kwargs is 100% welcomed as it makes the API more clear and readable, but it seems to have missed those fields, and possibly others or in other functions?

It also appears that factorize currently doesn't pass svd-related fields correctly into factorize_svd (see here, the svd fields don't seemed to be used anywhere in fact.

Happy to create a PR to add those fields if you agree, just let me know.

Minimal code demonstrating the bug or unexpected behavior

This succeeds in ITensors v0.3.48 & NDTensors v0.2.15
But fails in ITensors v0.3.52 & NDTensors v0.2.23:

using ITensors
mps=randomMPS(Float64, [Index(2) for _ in 1:5])
replacebond!(mps, 2, mps[3]; which_decomp="svd", use_relative_cutoff=true, min_blockdim=1)

Error in v0.3.52:

ERROR: MethodError: no method matching replacebond!(::MPS, ::Int64, ::ITensor; which_decomp::String, use_relative_cutoff::Bool, min_blockdim::Int64)

Closest candidates are:
  replacebond!(::MPS, ::Int64, ::ITensor; normalize, swapsites, ortho, which_decomp, mindim, maxdim, cutoff, eigen_perturbation, svd_alg) got unsupported keyword arguments "use_relative_cutoff", "min_blockdim"

Version information

  • Output from versioninfo():
julia> versioninfo()
Julia Version 1.10.0
Commit 3120989f39b (2023-12-25 18:01 UTC)
Build Info:
  Official https://julialang.org/ release
Platform Info:
  OS: macOS (x86_64-apple-darwin22.4.0)
  CPU: 12 × Intel(R) Core(TM) i7-9750H CPU @ 2.60GHz
  WORD_SIZE: 64
  LIBM: libopenlibm
  LLVM: libLLVM-15.0.7 (ORCJIT, skylake)
  Threads: 1 on 12 virtual cores
@brian-dellabetta brian-dellabetta added bug Something isn't working ITensors Issues or pull requests related to the `ITensors` package. labels Jan 2, 2024
@mtfishman
Copy link
Member

Thanks for the report, I'm not surprised some keyword arguments were lost in that PR since it was a pretty big undertaking. A PR would be welcome!

@brian-dellabetta
Copy link
Contributor Author

#1300 submitted to resolve this issue, please lmk what you think!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working ITensors Issues or pull requests related to the `ITensors` package.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants