From 260dbcda9b10f69c9dae230544fb09f23e6cf59f Mon Sep 17 00:00:00 2001 From: lan496 Date: Sun, 2 Apr 2023 21:58:26 +0900 Subject: [PATCH 1/6] Validate type of MSG --- src/magnetic_spacegroup.c | 3 ++ test/test_magnetic_symmetry.cpp | 60 +++++++++++++++++++++++++++++++++ 2 files changed, 63 insertions(+) diff --git a/src/magnetic_spacegroup.c b/src/magnetic_spacegroup.c index ab517b36b..f0deba0a4 100644 --- a/src/magnetic_spacegroup.c +++ b/src/magnetic_spacegroup.c @@ -438,6 +438,9 @@ static int get_reference_space_group(Spacegroup **ref_sg, /* Determine type of MSG and generator of factor group of MSG over XSG */ type = get_magnetic_space_group_type(&representatives, magnetic_symmetry, sym_fsg->size, sym_xsg->size); + if (type == 0) { + goto err; + } debug_print("type=%d\n", type); /* Choose reference setting */ diff --git a/test/test_magnetic_symmetry.cpp b/test/test_magnetic_symmetry.cpp index 233f23125..3ffd00cd5 100644 --- a/test/test_magnetic_symmetry.cpp +++ b/test/test_magnetic_symmetry.cpp @@ -364,6 +364,66 @@ TEST(test_magnetic_symmetry, test_spg_get_symmetry_with_tensors_rough_symprec) { free(time_reversals); } +TEST(test_magnetic_symmetry, test_spgms_get_magnetic_dataset_high_mag_symprec) { + // https://github.com/spglib/spglib/issues/249 + double lattice[3][3] = { + {0.00000000, -5.00000000, -2.50000000}, + {0.00000000, 0.00000000, 4.33012702}, + {-4.05000000, 0.00000000, 0.00000000}, + }; + double positions[][3] = { + {0.50000000, 0.33333333, 0.33333333}, + {0.50000000, 0.66666667, 0.66666667}, + {0.00000000, 0.00000000, 0.00000000}, + {0.00000000, 0.00000000, 0.50000000}, + {0.00000000, 0.50000000, 0.50000000}, + {0.00000000, 0.50000000, 0.00000000}, + }; + int types[] = {1, 1, 1, 2, 2, 2}; + double tensors[] = { + -0.00200000, 0.00200000, 1.90000000, 0.00200000, 0.00000000, + 1.91100000, -0.00100000, 0.00200000, -2.23300000, -0.00000000, + -0.00000000, -0.06000000, 0.00000000, 0.00000000, -0.03200000, + 0.00000000, -0.00000000, -0.02900000, + }; + int num_atoms = 6; + + int max_size = num_atoms * 96; + + double symprec = 1e-5; + double mag_symprec = 1e-1; // with high mag_symprec + + int i, size; + int equivalent_atoms[3]; + double primitive_lattice[3][3]; + int(*rotations)[3][3]; + double(*translations)[3]; + int *spin_flips; + int *time_reversals; + + rotations = (int(*)[3][3])malloc(sizeof(int[3][3]) * max_size); + translations = (double(*)[3])malloc(sizeof(double[3]) * max_size); + spin_flips = (int *)malloc(sizeof(int *) * max_size); + time_reversals = (int *)malloc(sizeof(int *) * max_size); + + size = spgms_get_symmetry_with_site_tensors( + rotations, translations, equivalent_atoms, primitive_lattice, + spin_flips, max_size, lattice, positions, types, tensors, + 1 /* tensor_rank */, num_atoms, 1 /* with_time_reversal */, + 1 /* is_axial */, symprec, -1 /* angle_tolerance */, mag_symprec); + ASSERT_TRUE(size > 0); + + SpglibMagneticDataset *dataset; + spgms_get_magnetic_dataset(lattice, positions, types, tensors, 1, num_atoms, + 1, symprec, -1, mag_symprec); + + free(rotations); + free(translations); + free(spin_flips); + free(time_reversals); + if (dataset != NULL) spg_free_magnetic_dataset(dataset); +} + TEST(test_magnetic_symmetry, test_with_broken_symmetry) { // https://github.com/spglib/spglib/issues/194 // Part of "mp-806965" in the Materials Project database From 1dc7efd29f16c828fe8bed994ed8e88ffcd9c3ed Mon Sep 17 00:00:00 2001 From: lan496 Date: Sun, 2 Apr 2023 22:05:19 +0900 Subject: [PATCH 2/6] Fix example command to run single test --- test/README.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/test/README.md b/test/README.md index 22423a880..9d2f68e9e 100644 --- a/test/README.md +++ b/test/README.md @@ -12,9 +12,11 @@ For example, to run `test_symmetry_search.test_spg_get_symmetry` ```shell # at build/test -./CTests --gtest_filter=test_spg_get_symmetry +./CTests --gtest_filter=test_symmetry_search.test_spg_get_symmetry ``` +If you use debuggers like gdb or lldb, recompile with `CMAKE_BUILD_TYPE=Debug`. + ## How to add a new test file 1. Create `.cpp` From 93e22d723b2fec82e3f82be0985bba945a3f4b52 Mon Sep 17 00:00:00 2001 From: lan496 Date: Sun, 2 Apr 2023 22:09:18 +0900 Subject: [PATCH 3/6] Clarify that `get_magnetic_symmetry_dataset` may return None --- doc/python-spglib.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/doc/python-spglib.md b/doc/python-spglib.md index 694801ee0..2c0ae028c 100644 --- a/doc/python-spglib.md +++ b/doc/python-spglib.md @@ -595,8 +595,9 @@ Arguments - `mag_symprec`: See {ref}`py_get_magnetic_symmetry` - `is_axial`: See {ref}`py_get_magnetic_symmetry` -Returned `dataset` is a dictionary. +If successful, returned `dataset` is a dictionary. The description of its keys is given at {ref}`magnetic_spglib_dataset`. +If the magnetic symmetry search failed, `dataset` is `None`. ### `get_magnetic_spacegroup_type` From 2722601061f5adf30f6a964e241b7ba377846562 Mon Sep 17 00:00:00 2001 From: Kohei Shinohara Date: Thu, 6 Apr 2023 22:39:36 +0900 Subject: [PATCH 4/6] Fix to store returned dataset Co-authored-by: Cristian Le --- test/test_magnetic_symmetry.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/test/test_magnetic_symmetry.cpp b/test/test_magnetic_symmetry.cpp index 3ffd00cd5..735175947 100644 --- a/test/test_magnetic_symmetry.cpp +++ b/test/test_magnetic_symmetry.cpp @@ -414,8 +414,9 @@ TEST(test_magnetic_symmetry, test_spgms_get_magnetic_dataset_high_mag_symprec) { ASSERT_TRUE(size > 0); SpglibMagneticDataset *dataset; - spgms_get_magnetic_dataset(lattice, positions, types, tensors, 1, num_atoms, - 1, symprec, -1, mag_symprec); + dataset = + spgms_get_magnetic_dataset(lattice, positions, types, tensors, 1, + num_atoms, 1, symprec, -1, mag_symprec); free(rotations); free(translations); From 4973cf3067778581379b9e79397e6e8b2c219188 Mon Sep 17 00:00:00 2001 From: lan496 Date: Thu, 6 Apr 2023 23:00:40 +0900 Subject: [PATCH 5/6] Show construct type before validation in debugging --- src/magnetic_spacegroup.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/magnetic_spacegroup.c b/src/magnetic_spacegroup.c index f0deba0a4..eac710d0b 100644 --- a/src/magnetic_spacegroup.c +++ b/src/magnetic_spacegroup.c @@ -438,10 +438,10 @@ static int get_reference_space_group(Spacegroup **ref_sg, /* Determine type of MSG and generator of factor group of MSG over XSG */ type = get_magnetic_space_group_type(&representatives, magnetic_symmetry, sym_fsg->size, sym_xsg->size); + debug_print("type=%d\n", type); if (type == 0) { goto err; } - debug_print("type=%d\n", type); /* Choose reference setting */ /* For type-IV, use setting from Hall symbol of XSG. */ From b67fcc7625a2c89e669a1129e74e0d5dc28cbc05 Mon Sep 17 00:00:00 2001 From: lan496 Date: Thu, 6 Apr 2023 23:01:42 +0900 Subject: [PATCH 6/6] Add comments why these should be asserted --- test/test_magnetic_symmetry.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/test/test_magnetic_symmetry.cpp b/test/test_magnetic_symmetry.cpp index 735175947..188eaf016 100644 --- a/test/test_magnetic_symmetry.cpp +++ b/test/test_magnetic_symmetry.cpp @@ -411,12 +411,16 @@ TEST(test_magnetic_symmetry, test_spgms_get_magnetic_dataset_high_mag_symprec) { spin_flips, max_size, lattice, positions, types, tensors, 1 /* tensor_rank */, num_atoms, 1 /* with_time_reversal */, 1 /* is_axial */, symprec, -1 /* angle_tolerance */, mag_symprec); + // spgms_get_symmetry_with_site_tensors should return one or more symmetry + // operations ASSERT_TRUE(size > 0); SpglibMagneticDataset *dataset; dataset = spgms_get_magnetic_dataset(lattice, positions, types, tensors, 1, num_atoms, 1, symprec, -1, mag_symprec); + // MSG identification is failed with the too high mag_symprec + ASSERT_TRUE(dataset == NULL); free(rotations); free(translations);