Skip to content

Conversation

@VinceNeede
Copy link
Contributor

Description

Fixing a warning and an error revealed for julia 1.12-beta1. In particular:

  1. Warning: Explicitly reference NDTensors for Tensor constructor from ITensors
  2. Warning: Change Compat.get_num_threads to BLAS.get_num_threads since first one is deprecated.
  3. Error in contract_threads: threadid goes from nthreads(:interactive) .+ 1:nthreads(:default) as reported in nthreads. The error was not spotted previously case the number of interactive threads was defaulted to 0, but now the default value is 1 as reported in the News. Note that this fixes the error, but threadid documentation suggest to don't use it for indexing arrays.

All the modifications should be still compatible with julia 1.11.5.

Fixes #1647

How Has This Been Tested?

Please add tests that verify your changes to a file in the test directory.

Please give a summary of the tests that you added to verify your changes.

  • Test A
  • Test B

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.

@VinceNeede
Copy link
Contributor Author

VinceNeede commented Apr 19, 2025

The error was present for Algorithm"threaded_folds" too, but I think this case was not covered by tests. Is it true? should one be added?

@mtfishman
Copy link
Member

Thanks for looking into this. That fix for the threading code looks a bit hacky, and as you say we shouldn't be indexing with threadid() in the first place: https://julialang.org/blog/2023/07/PSA-dont-use-threadid.

Can you focus this PR on changing Tensor to NDTensors.Tensor and changing Compat.get_num_threads to BLAS.get_num_threads and we can look into the threading issue later?

If Algorithm"threaded_folds" doesn't seem to be used probably we can delete it, I think it was left over from me experimenting with different parallelization approaches. Though maybe that approach will be better for addressing the threadid() issue.

@VinceNeede
Copy link
Contributor Author

Rebased the branch to the commit without the modification of contract_blocks.

About the threadid, this was just to make the old implementation work for julia 1.12 without getting a BoundsError and not a full fix. Do you want me to open an issue on it to help in keeping it in mind?

@mtfishman
Copy link
Member

Thanks, I've started a PR that removes the use of threadid here: #1650

@mtfishman mtfishman changed the title Getting ready for julia 1.12 [ITensor] Qualify overloading NDTensors.Tensor Apr 20, 2025
@mtfishman mtfishman changed the title [ITensor] Qualify overloading NDTensors.Tensor [ITensors] Qualify overloading NDTensors.Tensor Apr 20, 2025
@mtfishman mtfishman merged commit 69433ec into ITensor:main Apr 20, 2025
1 of 2 checks passed
@VinceNeede VinceNeede deleted the Issue_1647 branch April 20, 2025 18:16
@VinceNeede
Copy link
Contributor Author

Perfect, thank you as always.

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.

[ITensors] [BUG] Warning for Tensor constructor for julia 1.12-beta

2 participants