[NDTensors] JLArrays Extension #1508

kmp5VT · 2024-06-21T16:29:04Z

Description

Introduce the JLArrays extension to test GPU for ITensors

Checklist:

Introduce JLArrays and all NDTensors tests pass with array backend
Potentially move function from GPU array specific libraries to GPUArraysCoreExt
Update ITensors suite for JLArray to test on GPU adjacent array on CPU

kmp5VT · 2024-06-21T21:08:00Z

There is an issue in JLArrays, there is a missing resize! function JuliaGPU/GPUArrays.jl#541

NDTensors/ext/NDTEnsorsJLArraysExt/NDTensorsJLArraysExt.jl

mtfishman · 2024-06-21T21:11:27Z

NDTensors/ext/NDTEnsorsJLArraysExt/set_types.jl

+TypeParameterAccessors.position(::Type{<:JLArray}, ::typeof(eltype)) = Position(1)
+TypeParameterAccessors.position(::Type{<:JLArray}, ::typeof(ndims)) = Position(2)


These shouldn't be needed since I made generic definitions in #1505.

Good point, I removed these definitions for all GPU backends. Thanks!

NDTensors/Project.toml

mtfishman · 2024-06-21T21:13:45Z

NDTensors/ext/NDTEnsorsJLArraysExt/set_types.jl

+function TypeParameterAccessors.default_type_parameters(::Type{<:JLArray})
+  return (Float64, 1)
+end


We could consider making this a generic AbstractArray definition.

This is something I was thinking about. I think one issue with that is wrappers don't always have ndims second for example

julia> a = Matrix{Float32}(undef, (2,2)) julia> typeof(transpose(a)) LinearAlgebra.Transpose{Float32, Matrix{Float32}}

Right, so then for those types we would require making a custom overload of TypeParameterAccessors.default_type_parameters, which would be required anyway.

Co-authored-by: Matt Fishman <mtfishman@users.noreply.github.com>

mtfishman · 2024-06-22T23:47:28Z

NDTensors/test/NDTensorsTestUtils/device_list.jl

+if "jlarrays" in ARGS || "all" in ARGS
+  Pkg.add("JLArrays")
+  using JLArrays
+end


I don't think there is a reason to not run the tests for JLArrays since they run on CPU anyway. So I think it should just be added as a normal test dependency, and inserted into the device list by default.

So I am seeing an issue with the GPU tests when I have JLArrays is in the Project.toml. To fix the problem, I moved JLArrays to [extra] and I add it to the project if isempty(ARGS) || "base" in ARGS. However, this would run into a problem if the test args is ["base","metal"]. Right now we aren't testing in that way, though if you would prefer, I can just include JLArrays if no GPUs are being tested, i.e. only allow JLArrays if isempty(ARGS). Let me know, thanks!

What's the issue you see? It would be best to try to address that issue and have JLArrays as a dependency in Project.toml and run by default in the tests as we planned, so I would prefer to try to aim for that rather than work around an issue with a more convoluted solution.

The issue is that GPUArrays does not compile during Pkg.test

ERROR: LoadError: UndefVarError: `SimplifyCFGPassOptions` not defined Stacktrace: [1] top-level scope @ ~/.julia/packages/GPUCompiler/nWT2N/src/optim.jl:57 [2] include(mod::Module, _path::String) @ Base ./Base.jl:495 [3] include(x::String) @ GPUCompiler ~/.julia/packages/GPUCompiler/nWT2N/src/GPUCompiler.jl:1 [4] top-level scope @ ~/.julia/packages/GPUCompiler/nWT2N/src/GPUCompiler.jl:37 [5] include @ Base ./Base.jl:495 [inlined] [6] include_package_for_output(pkg::Base.PkgId, input::String, depot_path::Vector{String}, dl_load_path::Vector{String}, load_path::Vector{String}, concrete_deps::Vector{Pair{Base.PkgId, UInt128}}, source::String) @ Base ./loading.jl:2216 [7] top-level scope @ stdin:3 in expression starting at /home/jenkins/workspace/ITensor_ITensors.jl_PR-1508@tmp/.julia/packages/GPUCompiler/nWT2N/src/optim.jl:56 in expression starting at /home/jenkins/workspace/ITensor_ITensors.jl_PR-1508@tmp/.julia/packages/GPUCompiler/nWT2N/src/GPUCompiler.jl:1 in expression starting at stdin:3 ERROR: LoadError: Failed to precompile GPUCompiler [61eb1bfa-7361-4325-ad38-22787b887f55] to "/home/jenkins/workspace/ITensor_ITensors.jl_PR-1508@tmp/.julia/compiled/v1.10/GPUCompiler/jl_xlhZZG".

However, I think I was able to fix the problem by moving using JLArrays to after the GPU using statements.

NDTensors/test/NDTensorsTestUtils/device_list.jl

codecov-commenter · 2024-06-23T21:52:50Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 77.38%. Comparing base (82cfd76) to head (c0ebacb).
Report is 23 commits behind head on main.

❗ Current head c0ebacb differs from pull request most recent head d6e675d

Please upload reports for the commit d6e675d 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    #1508      +/-   ##
==========================================
- Coverage   78.05%   77.38%   -0.68%     
==========================================
  Files         148      140       -8     
  Lines        9679     9103     -576     
==========================================
- Hits         7555     7044     -511     
+ Misses       2124     2059      -65

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

kmp5VT · 2024-06-23T21:57:24Z

NDTensors/ext/NDTensorsJLArraysExt/NDTensorsJLArraysExt.jl

@@ -0,0 +1,8 @@
+module NDTensorsJLArraysExt
+include("copyto.jl")


I mostly copied these from CUDA definitions. I am working to determine which functions are actually necessary.

kmp5VT · 2024-07-02T15:50:10Z

For some reason I am having an issue with SparseArrays with Julia 1.6. All 1.6 tests fail with a compat issue

   Resolving package versions...
Downstream tests for ITensor DMRG: Error During Test at /Users/kpierce/.julia/packages/SafeTestsets/raUNr/src/SafeTestsets.jl:30
  Got exception outside of a @test
  LoadError: LoadError: LoadError: LoadError: Unsatisfiable requirements detected for package SparseArrays [2f01184e]:
   SparseArrays [2f01184e] log:
   ├─possible versions are: 0.0.0 or uninstalled
   ├─SparseArrays [2f01184e] is fixed to version 0.0.0
   └─restricted to versions 1.6.0-1 by NDTensors [23ae76d9] — no versions left
     └─NDTensors [23ae76d9] log:
       ├─possible versions are: 0.3.39 or uninstalled
       ├─restricted to versions 0.3.34-0.3 by ITensors [9136182c], leaving only versions 0.3.39
       │ └─ITensors [9136182c] log:
       │   ├─possible versions are: 0.6.16 or uninstalled
       │   └─ITensors [9136182c] is fixed to version 0.6.16
       └─NDTensors [23ae76d9] is fixed to version 0.3.39

mtfishman · 2024-07-02T16:52:54Z

For some reason I am having an issue with SparseArrays with Julia 1.6. All 1.6 tests fail with a compat issue

That's strange, do you see that locally as well?

Since CUDA.jl doesn't support Julia 1.6 now anyway, maybe if there isn't an obvious solution to that we could just stop testing against Julia 1.6 in the Jenkins workflow and not worry about Julia 1.6 support for the GPU backends.

The latest version of Julia compatible with CUDA.jl v5 is Julia 1.8, what if we test against Julia 1.8 instead of Julia 1.6 in the Jenkins tests? If that works, I would vote for just doing that.

kmp5VT · 2024-07-03T08:15:46Z

For some reason I am having an issue with SparseArrays with Julia 1.6. All 1.6 tests fail with a compat issue

That's strange, do you see that locally as well?

Yes I was seeing this locally on CPU and GPU. I am not sure why NDTensors tests work on CI with version 1.6. I bumped the version for the CUDA test to 1.8

kmp5VT · 2024-07-03T08:21:06Z

Also JLArrays does not have JLMatrix defined in julia v1.6

julia> versioninfo()
Julia Version 1.6.7
Commit 3b76b25b64 (2022-07-19 15:11 UTC)
Platform Info:
  OS: macOS (x86_64-apple-darwin21.4.0)
  CPU: Apple M1 Max
  WORD_SIZE: 64
  LIBM: libopenlibm
  LLVM: libLLVM-11.0.1 (ORCJIT, westmere)

julia> using JLArrays

julia> JLArray.JLMatrix
ERROR: type UnionAll has no field JLMatrix
Stacktrace:
 [1] getproperty(x::Type, f::Symbol)
   @ Base ./Base.jl:28
 [2] top-level scope
   @ REPL[7]:1

kmp5VT · 2024-07-03T08:52:34Z

And JLArrays also has no resize! function in Julia 1.8

Running /Users/kpierce/.julia/dev/ITensors/NDTensors/test/test_blocksparse.jl
test device: jl, eltype: Float32: Error During Test at /Users/kpierce/.julia/dev/ITensors/NDTensors/test/test_blocksparse.jl:32
  Got exception outside of a @test
  MethodError: no method matching resize!(::JLArrays.JLArray{Float32, 1}, ::Int64)
  Closest candidates are:
    resize!(::Vector, ::Integer) at array.jl:1233
    resize!(::BitVector, ::Integer) at bitarray.jl:814
    resize!(::NDTensors.SmallVectors.SubMSmallVector, ::Integer) at ~/.julia/dev/ITensors/NDTensors/src/lib/SmallVectors/src/subsmallvector/subsmallvector.jl:69

kmp5VT added 5 commits June 21, 2024 12:25

Start adding JLArrays extension

fc3ca6c

Bump version

0c9aae6

format

506a071

Add JLArrays to test to see what we need to add in NDTensors

c4dad63

Add registryt to `TypeParameterAcessors

f23d305

mtfishman reviewed Jun 21, 2024

View reviewed changes

NDTensors/ext/NDTEnsorsJLArraysExt/NDTensorsJLArraysExt.jl Outdated Show resolved Hide resolved

mtfishman reviewed Jun 21, 2024

View reviewed changes

NDTensors/Project.toml Outdated Show resolved Hide resolved

mtfishman reviewed Jun 21, 2024

View reviewed changes

kmp5VT and others added 6 commits June 22, 2024 19:41

Fix spelling

822411c

remove unnecessary functions

e213bff

format

7068a63

Remove dir

b8b46a3

rename folder

fd0866e

[no ci] alphabetize libraries

2caf2d1

Co-authored-by: Matt Fishman <mtfishman@users.noreply.github.com>

mtfishman reviewed Jun 22, 2024

View reviewed changes

mtfishman mentioned this pull request Jun 23, 2024

[ITensors] Generalize diag_itensor constructors to AbstractVector #1510

Merged

kmp5VT and others added 3 commits June 23, 2024 22:06

Merge branch 'main' into kmp5/feature/JLArrays_extension

9ad5670

Add JLArrays as dep and move CUDA to extras

0f1f277

Moving to make JLArrays always run

95881d5

mtfishman reviewed Jun 23, 2024

View reviewed changes

NDTensors/test/NDTensorsTestUtils/device_list.jl Outdated Show resolved Hide resolved

kmp5VT added 2 commits June 23, 2024 17:28

Add cuda to see if theres still issues on Jenkins (my machine is fine)

0acdaec

Fix import

a4687bd

Add Extension functions to JLArrays

cdee270

kmp5VT commented Jun 23, 2024

View reviewed changes

kmp5VT added 3 commits June 24, 2024 17:59

Merge branch 'main' into kmp5/feature/JLArrays_extension

d0b6a2f

Merge branch 'main' into kmp5/feature/JLArrays_extension

d6e675d

Fix the linear algebra and add jl to base tests

bdacf0e

kmp5VT added 3 commits June 27, 2024 18:44

format

d2be3b3

Try activate before update registry

9428514

Merge branch 'main' into kmp5/feature/JLArrays_extension

b3a2d87

kmp5VT marked this pull request as draft June 27, 2024 21:16

kmp5VT added 8 commits June 27, 2024 23:27

Move cuda back to deps

9165664

There is some issues with JLArrays on lower versions of Julia

1cd131d

Merge branch 'main' into kmp5/feature/JLArrays_extension

6654671

add back cuda

8a32daf

Having JLArrays when testing GPUs creates an issue in test

ad02066

Not using JLArrays in early versions of Julia

8534859

Move CUDA to extra

e2aa194

Add JLArrays back to deps

1078405

Bump CUDA test from 1.6 to 1.8

f47cf11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[NDTensors] JLArrays Extension #1508

[NDTensors] JLArrays Extension #1508

kmp5VT commented Jun 21, 2024 •

edited

Loading

kmp5VT commented Jun 21, 2024 •

edited

Loading

mtfishman Jun 21, 2024

kmp5VT Jun 22, 2024

mtfishman Jun 21, 2024

kmp5VT Jun 22, 2024

mtfishman Jun 22, 2024

mtfishman Jun 22, 2024

kmp5VT Jul 2, 2024 •

edited

Loading

mtfishman Jul 2, 2024

kmp5VT Jul 2, 2024

codecov-commenter commented Jun 23, 2024 •

edited

Loading

kmp5VT Jun 23, 2024

kmp5VT commented Jul 2, 2024

mtfishman commented Jul 2, 2024

kmp5VT commented Jul 3, 2024

kmp5VT commented Jul 3, 2024

kmp5VT commented Jul 3, 2024

		TypeParameterAccessors.position(::Type{<:JLArray}, ::typeof(eltype)) = Position(1)
		TypeParameterAccessors.position(::Type{<:JLArray}, ::typeof(ndims)) = Position(2)

		@@ -0,0 +1,8 @@
		module NDTensorsJLArraysExt
		include("copyto.jl")

[NDTensors] JLArrays Extension #1508

Are you sure you want to change the base?

[NDTensors] JLArrays Extension #1508

Conversation

kmp5VT commented Jun 21, 2024 • edited Loading

Description

Checklist:

kmp5VT commented Jun 21, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kmp5VT Jul 2, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov-commenter commented Jun 23, 2024 • edited Loading

Codecov Report

Choose a reason for hiding this comment

kmp5VT commented Jul 2, 2024

mtfishman commented Jul 2, 2024

kmp5VT commented Jul 3, 2024

kmp5VT commented Jul 3, 2024

kmp5VT commented Jul 3, 2024

kmp5VT commented Jun 21, 2024 •

edited

Loading

kmp5VT commented Jun 21, 2024 •

edited

Loading

kmp5VT Jul 2, 2024 •

edited

Loading

codecov-commenter commented Jun 23, 2024 •

edited

Loading