Fix edge-case in split_connected_components

Author: Liozou

Project.toml

@@ -1,7 +1,7 @@
 name = "PeriodicGraphs"
 uuid = "18c5b727-b240-4874-878a-f2e242435bab"
 authors = ["Lionel Zoubritzky lionel.zoubritzky@gmail.com"]
-version = "1.0.0"
+version = "1.0.1"
 
 [deps]
 Graphs = "86223c79-3864-5bf0-83f7-82e725a168b6"
@@ -11,6 +11,7 @@ StaticArrays = "90137ffa-7385-5640-81b9-e52037218182"
 
 [compat]
 Graphs = "1.3"
+LinearAlgebra = "1.10"
 PrecompileTools = "1"
 StaticArrays = "0.12, 1.0"
 julia = "1.6"

src/algorithms/dimensionality.jl

@@ -352,14 +352,22 @@ function _explore_one_component!(expected, encountered, visited, graph::Periodic
     end
     vec::Vector{SVector{N,Int}} = collect(recordedperiodicities)
     catenationmat, d = normal_basis(vec)
+    detmat = Int(det(catenationmat))
+    if detmat < 0 && N ≥ 2
+        # Swap the two first columns to make the determinant positive.
+        # This is not crucial to the algorithm, but it yields simpler offsets in the end.
+        for i in 1:N
+            catenationmat[i,1], catenationmat[i,2] = catenationmat[i,2], catenationmat[i,1]
+        end
+    end
     invmcatenationmat = inv(SMatrix{N,N,Rational{Int},N*N}(catenationmat))
     for (j, (src, (dst, ofs))) in enumerate(newedges)
         newedges[j] = PeriodicEdge{N}(src, dst, Int.(invmcatenationmat*ofs))
     end
 
     idx = minimum(keys(component))
     @assert expected[idx] == 0
-    expected[idx] = Int(det(catenationmat))
+    expected[idx] = abs(detmat)
     encountered[idx] = (PeriodicGraph{N}(length(Q), newedges), [OffsetVertexIterator(nullofs, Q)], catenationmat, d)
     idx
 end
@@ -431,8 +439,18 @@ function split_connected_components(graph::PeriodicGraph{N}) where N
             refmap = first(vmaps).nlist
             invmat = inv(SMatrix{N,N,Rational{Int},N*N}(mat))
             dumb_indices = findall(i -> begin onei = zeros(Int, N); onei[i] = 1; mat * onei == onei end, 1:N)
-            for h in 1:1000
-                x = reverse_hash_position(h, Val{N}())
+            h = 0
+            MAXh = 1000
+            oneaxes = ntuple(i -> SVector{N,Int}(ntuple(j -> Int(j == i), Val(N))), Val(N))
+            while true
+                h += 1
+                x = if h ≤ MAXh
+                    reverse_hash_position(h, Val{N}())
+                else
+                    hh = h - MAXh
+                    u, v = fldmod1(hh, N)
+                    NtoZ(u)*oneaxes[v]
+                end
                 all(i -> iszero(x[i]), dumb_indices) || continue
                 attempt_x = true
                 for ofsvmap in vmaps
@@ -459,7 +477,7 @@ function split_connected_components(graph::PeriodicGraph{N}) where N
                 length(vmaps) == ex && break
             end
         end
-        length(vmaps) < ex && error("Could not complete the splitting of connected components")
+        @assert length(vmaps) == ex
     end
     encountered[keep]
 end

test/Project.toml

@@ -7,7 +7,7 @@ StaticArrays = "90137ffa-7385-5640-81b9-e52037218182"
 Test = "8dfed614-e22c-5e08-85e1-65c5234f0b40"
 
 [compat]
-Aqua = "0.5"
+Aqua = "0.8"
 Combinatorics = "1.0"
 Graphs = "1.3"
 StaticArrays = "0.12, 1.0"

test/runtests.jl

@@ -746,6 +746,20 @@ end
     @test length(vmaps) == 2
     @test vmaps[1] == PeriodicVertex2D[(1, (0,0)), (3, (-1,0)), (4, (-1,1)), (2, (-1,1))]
     @test vmaps[2] == PeriodicVertex2D[(1, (1,-1)), (3, (0,-1)), (4, (0,0)), (2, (0,0))]
+
+    u2D = PeriodicGraph("2   1 1  0 2   2 2  0 1   2 2  1 0")
+    splits = split_connected_components(u2D)
+    @test length(splits) == 2
+    subgraph, vmaps, mat, dim = splits[1]
+    @test dim == 1
+    @test mat == SA[1 0; 0 2]
+    @test subgraph == PeriodicGraph("2 1 1 0 1")
+    @test vmaps == [PeriodicVertex2D[(1, (0,0))], PeriodicVertex2D[(1, (0,-1))]]
+    subgraph, vmaps, mat, dim = splits[2]
+    @test dim == 2
+    @test mat == SA[1 0; 0 1]
+    @test subgraph == PeriodicGraph("2 1 1 0 1 1 1 1 0")
+    @test vmaps == [PeriodicVertex2D[(2, (0,0))]]
 end
 
 @testset "Dimension change" begin