Merge pull request #143 from singularitti:cleanuo

Fix API for `get_spacegroup_type_from_symmetry` & `get_hall_number_from_symmetry`
singularitti · Aug 16, 2023 · 72229fa · 72229fa
2 parents 7c6849f + 72d9bd9
commit 72229fa
Show file tree

Hide file tree

Showing 2 changed files with 66 additions and 34 deletions.
diff --git a/src/symmetry.jl b/src/symmetry.jl
@@ -83,7 +83,7 @@ function get_symmetry(cell::AbstractCell, symprec=1e-5)
     # See https://github.com/spglib/spglib/blob/42527b0/python/spglib/spglib.py#L270
     max_size = 48num_atom  # Num of symmetry operations = order of the point group of the space group × num of lattice points
     rotations = Array{Cint,3}(undef, 3, 3, max_size)
-    translations = Array{Cdouble,2}(undef, 3, max_size)  # C is row-major order, but Julia is column-major order
+    translations = Matrix{Cdouble}(undef, 3, max_size)  # C is row-major order, but Julia is column-major order
     num_sym = @ccall libsymspg.spg_get_symmetry(
         rotations::Ptr{Cint},
         translations::Ptr{Cdouble},
@@ -127,7 +127,7 @@ function get_symmetry_from_database(hall_number)
     # The maximum number of symmetry operations is 192, see https://github.com/spglib/spglib/blob/77a8e5d/src/spglib.h#L382
     @assert 1 <= hall_number <= 530
     rotations = Array{Cint,3}(undef, 3, 3, 192)
-    translations = Array{Cdouble,2}(undef, 3, 192)
+    translations = Matrix{Cdouble}(undef, 3, 192)
     num_sym = @ccall libsymspg.spg_get_symmetry_from_database(
         rotations::Ptr{Cint}, translations::Ptr{Cdouble}, hall_number::Cint
     )::Cint
@@ -169,7 +169,7 @@ function get_dataset(cell::AbstractCell, symprec=1e-5)
         natoms(cell)::Cint,
         symprec::Cdouble,
     )::Ptr{SpglibDataset}
-    if ptr == C_NULL
+    if ptr == C_NULL  # See https://github.com/spglib/spglib/blob/v2.1.0-rc2/python/spglib/spglib.py#L498-L504
         check_error()
     else
         dataset = unsafe_load(ptr)
@@ -282,18 +282,19 @@ end
 """
     get_spacegroup_type(hall_number)
 
-Translate Hall number to space group type information.
+Translate Hall number to space group information.
 """
 function get_spacegroup_type(hall_number)
     @assert 1 <= hall_number <= 530
     spgtype = @ccall libsymspg.spg_get_spacegroup_type(
         hall_number::Cint
     )::SpglibSpacegroupType
+    check_error()
     return convert(SpacegroupType, spgtype)
 end
 
 """
-    get_spacegroup_type_from_symmetry(cell::AbstractCell, symprec=1e-5)
+    get_spacegroup_type_from_symmetry(rotations, translations, lattice::Lattice, symprec=1e-5)
 
 Return space-group type information from symmetry operations.
 
@@ -316,26 +317,35 @@ julia> lattice = Lattice([
 ]);
 ```
 """
-function get_spacegroup_type_from_symmetry(cell::AbstractCell, symprec=1e-5)
-    rotations, translations = get_symmetry(cell, symprec)
+function get_spacegroup_type_from_symmetry(
+    rotations, translations, lattice::Lattice, symprec=1e-5
+)
+    if length(rotations) != length(translations)
+        throw(DimensionMismatch("the numbers of rotations and translations are different!"))
+    end
     num_sym = length(translations)
-    rotations, translations = cat(transpose.(rotations)...; dims=3),
-    reduce(hcat, translations)
-    lattice, _, _ = _expand_cell(cell)
+    rotations = convert(Array{Cint,3}, cat(transpose.(rotations)...; dims=3))
+    translations = convert(Matrix{Cdouble}, reduce(hcat, translations))
+    lattice = Base.cconvert(Matrix{Cdouble}, transpose(lattice))   # `transpose` must before `cconvert`!
     spgtype = @ccall libsymspg.spg_get_spacegroup_type_from_symmetry(
         rotations::Ptr{Cint},
         translations::Ptr{Cdouble},
         num_sym::Cint,
         lattice::Ptr{Cdouble},
         symprec::Cdouble,
     )::SpglibSpacegroupType
+    check_error()
     return convert(SpacegroupType, spgtype)
 end
 
 """
-    get_hall_number_from_symmetry(cell::AbstractCell, symprec=1e-5)
+    get_hall_number_from_symmetry(rotations, translations, symprec=1e-5)
+
+Return one Hall number corresponding to a space group of the given set of symmetry operations.
 
-Obtain `hall_number` from the set of symmetry operations.
+When multiple Hall numbers exist for the space group, the smallest one
+(the first description of the space-group type in International Tables for Crystallography)
+is chosen.
 
 This is expected to work well for the set of symmetry operations whose
 distortion is small. The aim of making this feature is to find
@@ -348,15 +358,14 @@ fractional coordinates and so it should be small like `1e-5`.
 !!! warning
     This function will be replaced by [`get_spacegroup_type_from_symmetry`](@ref).
 """
-function get_hall_number_from_symmetry(cell::AbstractCell, symprec=1e-5)
-    rotations, translations = get_symmetry(cell, symprec)
+function get_hall_number_from_symmetry(rotations, translations, symprec=1e-5)
     num_sym = length(translations)
-    rotations, translations = cat(transpose.(rotations)...; dims=3),
-    reduce(hcat, translations)
+    rotations = convert(Array{Cint,3}, cat(transpose.(rotations)...; dims=3))
+    translations = convert(Matrix{Cdouble}, reduce(hcat, translations))
     hall_number = @ccall libsymspg.spg_get_hall_number_from_symmetry(
         rotations::Ptr{Cint}, translations::Ptr{Cdouble}, num_sym::Cint, symprec::Cdouble
     )::Cint
-    check_error()
+    check_error()  # The Python code does not check errors here? https://github.com/spglib/spglib/blob/v2.1.0-rc2/python/spglib/spglib.py#L1588-L1605
     return hall_number
 end
 

diff --git a/test/symmetry.jl b/test/symmetry.jl
@@ -201,7 +201,9 @@ end
         [0.5, 0.0, 0.5],
         [0.5, 0.0, 0.5],
     ]  # Compared with Python results
-    @test get_spacegroup_type_from_symmetry(cell, 1e-5) == SpacegroupType(
+    @test get_spacegroup_type_from_symmetry(
+        dataset.rotations, dataset.translations, Lattice(cell), 1e-5
+    ) == SpacegroupType(
         64,
         "Cmce",
         "C 2/m 2/c 2_1/e",
@@ -215,7 +217,8 @@ end
         19,
         "mmmC",
     )  # Compared with Python results
-    @test get_hall_number_from_symmetry(cell, 1e-5) == dataset.hall_number
+    @test get_hall_number_from_symmetry(dataset.rotations, dataset.translations, 1e-5) ==
+        dataset.hall_number
     @test get_multiplicity(cell, 1e-5) == length(dataset.translations)
     @test get_dataset_with_hall_number(cell, dataset.hall_number) == dataset
     @test get_schoenflies(cell, 1e-5) == "D2h^18"
@@ -333,7 +336,9 @@ end
     @test dataset.std_types == [1, 1, 2, 2, 2, 2]  # 14, 14, 8, 8, 8, 8
     @test dataset.std_mapping_to_primitive == [0, 1, 2, 3, 4, 5]
     @test dataset.pointgroup_symbol == "4/mmm"
-    @test get_spacegroup_type_from_symmetry(cell, 1e-5) == SpacegroupType(
+    @test get_spacegroup_type_from_symmetry(
+        dataset.rotations, dataset.translations, Lattice(cell), 1e-5
+    ) == SpacegroupType(
         136,
         "P4_2/mnm",
         "P 4_2/m 2_1/n 2/m",
@@ -347,7 +352,8 @@ end
         36,
         "4/mmmP",
     )  # Compared with Python results
-    @test get_hall_number_from_symmetry(cell, 1e-5) == dataset.hall_number
+    @test get_hall_number_from_symmetry(dataset.rotations, dataset.translations, 1e-5) ==
+        dataset.hall_number
     @test get_multiplicity(cell, 1e-5) == length(dataset.translations)
     @test get_dataset_with_hall_number(cell, dataset.hall_number) == dataset
     @test get_schoenflies(cell, 1e-5) == "D4h^14"
@@ -438,9 +444,11 @@ end
     @test get_symmetry(cell) == (dataset.rotations, dataset.translations)
     @test get_symmetry_from_database(dataset.hall_number)[1] == dataset.rotations
     @test get_symmetry_from_database(dataset.hall_number)[2] == dataset.translations
-    @test get_spacegroup_type_from_symmetry(cell, 1e-5) ==
-        SpacegroupType(1, "P1", "P 1", "P 1", "C1^1", 1, "P 1", "", "1", "C1", 1, "1P")  # Compared with Python results
-    @test get_hall_number_from_symmetry(cell, 1e-5) == dataset.hall_number
+    @test get_spacegroup_type_from_symmetry(
+        dataset.rotations, dataset.translations, Lattice(cell), 1e-5
+    ) == SpacegroupType(1, "P1", "P 1", "P 1", "C1^1", 1, "P 1", "", "1", "C1", 1, "1P")  # Compared with Python results
+    @test get_hall_number_from_symmetry(dataset.rotations, dataset.translations, 1e-5) ==
+        dataset.hall_number
     @test get_multiplicity(cell, 1e-5) == length(dataset.translations)
     @test get_dataset_with_hall_number(cell, dataset.hall_number) == dataset
     @test get_schoenflies(cell, 1e-5) == "C1^1"
@@ -573,7 +581,9 @@ end
     end
     @test dataset.std_mapping_to_primitive == [0, 1]
     @test dataset.pointgroup_symbol == "-3m"
-    @test get_spacegroup_type_from_symmetry(cell, 1e-5) == SpacegroupType(
+    @test get_spacegroup_type_from_symmetry(
+        dataset.rotations, dataset.translations, Lattice(cell), 1e-5
+    ) == SpacegroupType(
         164,
         "P-3m1",
         "P -3 2/m 1",
@@ -587,7 +597,8 @@ end
         49,
         "-3m1P",
     )  # Compared with Python results
-    @test get_hall_number_from_symmetry(cell, 1e-5) == dataset.hall_number
+    @test get_hall_number_from_symmetry(dataset.rotations, dataset.translations, 1e-5) ==
+        dataset.hall_number
     @test get_multiplicity(cell, 1e-5) == length(dataset.translations)
     @test get_dataset_with_hall_number(cell, dataset.hall_number) == dataset
     @test get_schoenflies(cell, 1e-5) == "D3d^3"
@@ -1058,7 +1069,9 @@ end
     end
     @test dataset.std_mapping_to_primitive == [0, 1, 0, 1, 0, 1, 0, 1]
     @test dataset.pointgroup_symbol == "m-3m"
-    @test get_spacegroup_type_from_symmetry(cell, 1e-5) == SpacegroupType(
+    @test get_spacegroup_type_from_symmetry(
+        dataset.rotations, dataset.translations, Lattice(cell), 1e-5
+    ) == SpacegroupType(
         227,
         "Fd-3m",
         "F 4_1/d -3 2/m",
@@ -1072,7 +1085,8 @@ end
         72,
         "m-3mF",
     )  # Compared with Python results
-    @test get_hall_number_from_symmetry(cell, 1e-5) == dataset.hall_number
+    @test get_hall_number_from_symmetry(dataset.rotations, dataset.translations, 1e-5) ==
+        dataset.hall_number
     @test get_multiplicity(cell, 1e-5) == length(dataset.translations)
     @test get_dataset_with_hall_number(cell, dataset.hall_number) == dataset
     @test get_schoenflies(cell, 1e-5) == "Oh^7"
@@ -1244,7 +1258,9 @@ end
     end
     @test dataset.std_mapping_to_primitive == [0, 1, 0, 1, 0, 1, 0, 1]
     @test dataset.pointgroup_symbol == "m-3m"
-    @test get_spacegroup_type_from_symmetry(cell, 1e-5) == SpacegroupType(
+    @test get_spacegroup_type_from_symmetry(
+        dataset.rotations, dataset.translations, Lattice(cell), 1e-5
+    ) == SpacegroupType(
         227,
         "Fd-3m",
         "F 4_1/d -3 2/m",
@@ -1258,7 +1274,8 @@ end
         72,
         "m-3mF",
     )  # Compared with Python results
-    @test get_hall_number_from_symmetry(cell, 1e-5) == dataset.hall_number
+    @test get_hall_number_from_symmetry(dataset.rotations, dataset.translations, 1e-5) ==
+        dataset.hall_number
     @test get_multiplicity(cell, 1e-5) == length(dataset.translations)
     @test get_dataset_with_hall_number(cell, dataset.hall_number) == dataset
     @test get_schoenflies(cell, 1e-5) == "Oh^7"
@@ -1373,7 +1390,9 @@ end
             dataset.std_rotation_matrix * std_lattice_before_idealization
     end
     @test dataset.std_mapping_to_primitive == 0:2
-    @test get_spacegroup_type_from_symmetry(cell, 1e-5) == SpacegroupType(
+    @test get_spacegroup_type_from_symmetry(
+        dataset.rotations, dataset.translations, Lattice(cell), 1e-5
+    ) == SpacegroupType(
         191,
         "P6/mmm",
         "P 6/m 2/m 2/m",
@@ -1387,7 +1406,8 @@ end
         58,
         "6/mmm",
     )  # Compared with Python results
-    @test get_hall_number_from_symmetry(cell, 1e-5) == dataset.hall_number
+    @test get_hall_number_from_symmetry(dataset.rotations, dataset.translations, 1e-5) ==
+        dataset.hall_number
     @test get_multiplicity(cell, 1e-5) == length(dataset.translations)
     @test get_dataset_with_hall_number(cell, dataset.hall_number) == dataset
     @test get_schoenflies(cell, 1e-5) == "D6h^1"
@@ -1501,7 +1521,9 @@ end
             dataset.std_rotation_matrix * std_lattice_before_idealization
     end
     @test dataset.std_mapping_to_primitive == 0:3  # FIXME: should I +1?
-    @test get_spacegroup_type_from_symmetry(cell, 1e-5) == SpacegroupType(
+    @test get_spacegroup_type_from_symmetry(
+        dataset.rotations, dataset.translations, Lattice(cell), 1e-5
+    ) == SpacegroupType(
         186,
         "P6_3mc",
         "P 6_3 m c",
@@ -1515,7 +1537,8 @@ end
         55,
         "6mmP",
     )  # Compared with Python results
-    @test get_hall_number_from_symmetry(cell, 1e-5) == dataset.hall_number
+    @test get_hall_number_from_symmetry(dataset.rotations, dataset.translations, 1e-5) ==
+        dataset.hall_number
     @test get_multiplicity(cell, 1e-5) == length(dataset.translations)
     @test get_dataset_with_hall_number(cell, dataset.hall_number) == dataset
     @test get_schoenflies(cell, 1e-5) == "C6v^4"