Fix some incorrect dimension permutations in getindex/setindex! (#26)

Author: mtfishman

Project.toml

@@ -1,7 +1,7 @@
 name = "NamedDimsArrays"
 uuid = "60cbd0c0-df58-4cb7-918c-6f5607b73fde"
 authors = ["ITensor developers <[email protected]> and contributors"]
-version = "0.3.10"
+version = "0.3.11"
 
 [deps]
 Adapt = "79e6a3ab-5dfb-504d-930d-738a2a938a0e"

src/abstractnameddimsarray.jl

@@ -400,8 +400,7 @@ function Base.getindex(
   a::AbstractNamedDimsArray, I1::AbstractNamedInteger, Irest::AbstractNamedInteger...
 )
   I = (I1, Irest...)
-  # TODO: Check if this permuation should be inverted.
-  perm = getperm(name.(nameddimsindices(a)), name.(I))
+  perm = getperm(name.(I), name.(nameddimsindices(a)))
   # TODO: Throw a `NameMismatch` error.
   @assert isperm(perm)
   I = map(p -> I[p], perm)
@@ -446,8 +445,7 @@ function Base.setindex!(
   a::AbstractNamedDimsArray, value, I1::AbstractNamedInteger, Irest::AbstractNamedInteger...
 )
   I = flatten_namedinteger.((I1, Irest...))
-  # TODO: Check if this permuation should be inverted.
-  perm = getperm(name.(nameddimsindices(a)), name.(I))
+  perm = getperm(name.(I), name.(nameddimsindices(a)))
   # TODO: Throw a `NameMismatch` error.
   @assert isperm(perm)
   I = map(p -> I[p], perm)
@@ -523,8 +521,7 @@ end
 
 function Base.view(a::AbstractNamedDimsArray, I1::NamedViewIndex, Irest::NamedViewIndex...)
   I = (I1, Irest...)
-  # TODO: Check if this permuation should be inverted.
-  perm = getperm(name.(nameddimsindices(a)), name.(I))
+  perm = getperm(name.(I), name.(nameddimsindices(a)))
   # TODO: Throw a `NameMismatch` error.
   @assert isperm(perm)
   I = map(p -> I[p], perm)
@@ -604,7 +601,6 @@ end
 
 function aligndims(a::AbstractArray, dims)
   new_nameddimsindices = to_nameddimsindices(a, dims)
-  # TODO: Check this permutation is correct (it may be the inverse of what we want).
   perm = Tuple(getperm(nameddimsindices(a), new_nameddimsindices))
   isperm(perm) || throw(
     NameMismatch(
@@ -616,7 +612,6 @@ end
 
 function aligneddims(a::AbstractArray, dims)
   new_nameddimsindices = to_nameddimsindices(a, dims)
-  # TODO: Check this permutation is correct (it may be the inverse of what we want).
   perm = getperm(nameddimsindices(a), new_nameddimsindices)
   isperm(perm) || throw(
     NameMismatch(

test/Project.toml

@@ -3,6 +3,7 @@ Adapt = "79e6a3ab-5dfb-504d-930d-738a2a938a0e"
 Aqua = "4c88cf16-eb10-579e-8560-4a9242c79595"
 BlockArrays = "8e7c35d0-a365-5155-bbbb-fb81a777f24e"
 BlockSparseArrays = "2c9a651f-6452-4ace-a6ac-809f4280fbb4"
+Combinatorics = "861a8166-3701-5b0c-9a16-15d98fcdc6aa"
 LinearAlgebra = "37e2e46d-f89d-539d-b4ee-838fcccc9c8e"
 NamedDimsArrays = "60cbd0c0-df58-4cb7-918c-6f5607b73fde"
 SafeTestsets = "1bc83da4-3b8d-516f-aca4-4fe02f6d838f"

test/basics/test_basics.jl

@@ -1,3 +1,4 @@
+using Combinatorics: Combinatorics
 using NamedDimsArrays:
   NamedDimsArrays,
   AbstractNamedDimsArray,
@@ -198,6 +199,23 @@ using Test: @test, @test_throws, @testset
     c = nameddims(Array{elt}(undef, 2, 3), (:i, :j))
     c .= a .+ 2 .* b
     @test dename(c, (:i, :j)) ≈ dename(a, (:i, :j)) + 2 * dename(b, (:i, :j))
+
+    # Regression test for proper permutations.
+    a = nameddims(randn(elt, 2, 3, 4), (:i, :j, :k))
+    I = (:i => 2, :j => 3, :k => 4)
+    for I′ in Combinatorics.permutations(I)
+      @test a[I′...] == a[2, 3, 4]
+      a′ = copy(a)
+      a′[I′...] = zero(Bool)
+      @test iszero(a′[2, 3, 4])
+    end
+    I = (:i => 2, :j => 2:3, :k => 4)
+    for I′ in Combinatorics.permutations(I)
+      @test a[I′...] == a[2, 2:3, 4]
+      ## TODO: This is broken, investigate.
+      ## a′[I′...] = zeros(Bool, 2)
+      ## @test iszero(a′[2, 2:3, 4])
+    end
   end
   @testset "Shorthand constructors (eltype=$elt)" for elt in (
     Float32, ComplexF32, Float64, ComplexF64