diff --git a/python/src/configuration.cpp b/python/src/configuration.cpp index 6a86f7b..864973c 100644 --- a/python/src/configuration.cpp +++ b/python/src/configuration.cpp @@ -629,6 +629,19 @@ PYBIND11_MODULE(_configuration, m) { R"pbdoc( Entry in a :class:`~libcasm.configuration.SupercellSet` + Notes + ----- + + - :class:`~libcasm.configuration.SupercellRecord` comparison + (``==``, ``!=``, ``<``, etc.) is based on supercell geometry only, + ignoring ``supercell_name``. This is equivalent to comparing + ``self.supercell == other.supercell``. + - To compare a :class:`~libcasm.configuration.Supercell` with a + :class:`~libcasm.configuration.SupercellRecord`, compare the + supercells directly: + ``supercell == record.supercell``. Comparing + ``supercell == record`` will warn and return ``False``. + )pbdoc"); // ConfigurationSet -- declare class @@ -769,7 +782,26 @@ PYBIND11_MODULE(_configuration, m) { // ConfigurationRecord -- declare class py::class_ pyConfigurationRecord( m, "ConfigurationRecord", R"pbdoc( - Entry in a :class:`~libcasm.configuration.ConfigurationSet` + Entry in a :class:`~libcasm.configuration.ConfigurationSet` + + Notes + ----- + + - :class:`~libcasm.configuration.ConfigurationRecord` comparison + (``==``, ``!=``, ``<``, etc.) is based on DoF values only, ignoring + ``configuration_name``. This is equivalent to comparing + ``self.configuration == other.configuration``. + - To compare a :class:`~libcasm.configuration.Configuration` with a + :class:`~libcasm.configuration.ConfigurationRecord`, compare the + configurations directly: + ``configuration == record.configuration``. Comparing + ``configuration == record`` will warn and return ``False``. + - To compare a :class:`~libcasm.configuration.ConfigurationWithProperties` + with a :class:`~libcasm.configuration.ConfigurationRecord`, compare the + configurations directly: + ``configuration_with_properties.configuration == record.configuration``. + Comparing ``configuration_with_properties == record`` will warn and + return ``False``. )pbdoc"); @@ -832,6 +864,17 @@ PYBIND11_MODULE(_configuration, m) { - Configuration may be copied with :func:`Configuration.copy `, `copy.copy`, or `copy.deepcopy`. + - To compare a :class:`~libcasm.configuration.Configuration` with a + :class:`~libcasm.configuration.ConfigurationRecord`, compare the + configurations directly: + ``configuration == record.configuration``. Comparing + ``configuration == record`` will warn and return ``False``. + - To compare a :class:`~libcasm.configuration.Configuration` with a + :class:`~libcasm.configuration.ConfigurationWithProperties`, compare + the configurations directly: + ``configuration == configuration_with_properties.configuration``. + Comparing ``configuration == configuration_with_properties`` will + warn and return ``False``. .. rubric:: Special Methods @@ -897,6 +940,18 @@ PYBIND11_MODULE(_configuration, m) { - ConfigurationWithProperties may be copied with :func:`ConfigurationWithProperties.copy `, `copy.copy`, or `copy.deepcopy`. + - To compare a :class:`~libcasm.configuration.Configuration` with a + :class:`~libcasm.configuration.ConfigurationWithProperties`, compare + the configurations directly: + ``configuration_with_properties.configuration == configuration``. + Comparing ``configuration == configuration_with_properties`` will + warn and return ``False``. + - To compare a :class:`~libcasm.configuration.ConfigurationRecord` with a + :class:`~libcasm.configuration.ConfigurationWithProperties`, compare + the configurations directly: + ``configuration_with_properties.configuration == record.configuration``. + Comparing ``record == configuration_with_properties`` will warn and + return ``False``. )pbdoc"); @@ -906,6 +961,15 @@ PYBIND11_MODULE(_configuration, m) { the symmetry representations needed for applying symmetry to :class:`~libcasm.configuration.Configuration` in that supercell. + Notes + ----- + + - To compare a :class:`~libcasm.configuration.Supercell` with a + :class:`~libcasm.configuration.SupercellRecord`, compare the + supercells directly: + ``supercell == record.supercell``. Comparing + ``supercell == record`` will warn and return ``False``. + )pbdoc") .def(py::init(&make_supercell), py::arg("prim"), py::arg("transformation_matrix_to_super").noconvert(), @@ -1365,6 +1429,28 @@ PYBIND11_MODULE(_configuration, m) { "" "True if supercells are not equal. Only supercells with the same " "prim can be compared.") + .def( + "__eq__", + [](std::shared_ptr const &, + config::SupercellRecord const &) -> bool { + py::module_::import("warnings") + .attr("warn")( + "Comparing a Supercell with a SupercellRecord directly. " + "Did you mean to compare supercell == record.supercell?"); + return false; + }, + py::arg("other")) + .def( + "__ne__", + [](std::shared_ptr const &, + config::SupercellRecord const &) -> bool { + py::module_::import("warnings") + .attr("warn")( + "Comparing a Supercell with a SupercellRecord directly. " + "Did you mean to compare supercell != record.supercell?"); + return true; + }, + py::arg("other")) .def_static( "from_dict", [](const nlohmann::json &data, @@ -1490,6 +1576,28 @@ PYBIND11_MODULE(_configuration, m) { .def(py::self >= py::self, "Sorts SupercellRecord.") .def(py::self == py::self, "Compare SupercellRecord.") .def(py::self != py::self, "Compare SupercellRecord.") + .def( + "__eq__", + [](config::SupercellRecord const &, + std::shared_ptr const &) -> bool { + py::module_::import("warnings") + .attr("warn")( + "Comparing a SupercellRecord with a Supercell directly. " + "Did you mean to compare record.supercell == supercell?"); + return false; + }, + py::arg("other")) + .def( + "__ne__", + [](config::SupercellRecord const &, + std::shared_ptr const &) -> bool { + py::module_::import("warnings") + .attr("warn")( + "Comparing a SupercellRecord with a Supercell directly. " + "Did you mean to compare record.supercell != supercell?"); + return true; + }, + py::arg("other")) .def( "copy", [](config::SupercellRecord const &self) { @@ -2147,6 +2255,17 @@ PYBIND11_MODULE(_configuration, m) { [](config::ConfigurationSet &m, config::ConfigurationRecord const &record) -> config::ConfigurationRecord const & { + auto name_it = m.find_by_name(record.configuration_name); + if (name_it != m.end() && + m.find(record.configuration) == m.end()) { + std::stringstream msg; + msg << "ConfigurationSet.add_record: a record with " + "configuration_name '" + << record.configuration_name + << "' already exists with different DoF. Overwriting."; + py::module_::import("warnings").attr("warn")(msg.str()); + m.erase_by_name(record.configuration_name); + } return *m.insert(record).first; }, py::return_value_policy::reference_internal, @@ -2171,6 +2290,17 @@ PYBIND11_MODULE(_configuration, m) { [](config::ConfigurationSet &m, config::ConfigurationRecord const &record) -> config::ConfigurationRecord const & { + auto name_it = m.find_by_name(record.configuration_name); + if (name_it != m.end() && + m.find(record.configuration) == m.end()) { + std::stringstream msg; + msg << "ConfigurationSet.add_record: a record with " + "configuration_name '" + << record.configuration_name + << "' already exists with different DoF. Overwriting."; + py::module_::import("warnings").attr("warn")(msg.str()); + m.erase_by_name(record.configuration_name); + } return *m.insert(record).first; }, py::return_value_policy::reference_internal, @@ -2404,7 +2534,28 @@ PYBIND11_MODULE(_configuration, m) { jsonParser json{data}; std::shared_ptr configurations = std::make_shared(); + + // Count config entries in the dict before loading + std::size_t n_entries = 0; + if (data.contains("supercells")) { + for (auto const &[scel_name, scel_data] : + data["supercells"].items()) { + n_entries += scel_data.size(); + } + } + from_json(*supercells, *configurations, json, supercells->prim()); + + if (configurations->size() != n_entries) { + std::stringstream msg; + msg << "ConfigurationSet.from_dict: the input dict contained " + << n_entries << " configurations, but only " + << configurations->size() + << " unique configurations were inserted " + << "(duplicates were discarded)."; + py::module_::import("warnings").attr("warn")(msg.str()); + } + return configurations; }, R"pbdoc( @@ -3221,6 +3372,52 @@ PYBIND11_MODULE(_configuration, m) { "True if configurations are equal, or approximately equal up the " "lattice tolerance if there continuous DoF. Only configurations " "with the same prim can be compared.") + .def( + "__eq__", + [](config::Configuration const &, + config::ConfigurationRecord const &) -> bool { + py::module_::import("warnings").attr("warn")( + "Comparing a Configuration with a ConfigurationRecord " + "directly. Did you mean to compare " + "configuration == record.configuration?"); + return false; + }, + py::arg("other")) + .def( + "__ne__", + [](config::Configuration const &, + config::ConfigurationRecord const &) -> bool { + py::module_::import("warnings").attr("warn")( + "Comparing a Configuration with a ConfigurationRecord " + "directly. Did you mean to compare " + "configuration != record.configuration?"); + return true; + }, + py::arg("other")) + .def( + "__eq__", + [](config::Configuration const &, + config::ConfigurationWithProperties const &) -> bool { + py::module_::import("warnings").attr("warn")( + "Comparing a Configuration with a ConfigurationWithProperties " + "directly. Did you mean to compare " + "configuration == " + "configuration_with_properties.configuration?"); + return false; + }, + py::arg("other")) + .def( + "__ne__", + [](config::Configuration const &, + config::ConfigurationWithProperties const &) -> bool { + py::module_::import("warnings").attr("warn")( + "Comparing a Configuration with a ConfigurationWithProperties " + "directly. Did you mean to compare " + "configuration != " + "configuration_with_properties.configuration?"); + return true; + }, + py::arg("other")) .def( "copy", [](config::Configuration const &self) { @@ -3506,6 +3703,54 @@ PYBIND11_MODULE(_configuration, m) { float: Scalar global property value. )pbdoc", py::arg("key")) + .def( + "__eq__", + [](config::ConfigurationWithProperties const &, + config::Configuration const &) -> bool { + py::module_::import("warnings").attr("warn")( + "Comparing a ConfigurationWithProperties with a " + "Configuration directly. Did you mean to compare " + "configuration_with_properties.configuration == " + "configuration?"); + return false; + }, + py::arg("other")) + .def( + "__ne__", + [](config::ConfigurationWithProperties const &, + config::Configuration const &) -> bool { + py::module_::import("warnings").attr("warn")( + "Comparing a ConfigurationWithProperties with a " + "Configuration directly. Did you mean to compare " + "configuration_with_properties.configuration != " + "configuration?"); + return true; + }, + py::arg("other")) + .def( + "__eq__", + [](config::ConfigurationWithProperties const &, + config::ConfigurationRecord const &) -> bool { + py::module_::import("warnings").attr("warn")( + "Comparing a ConfigurationWithProperties with a " + "ConfigurationRecord directly. Did you mean to compare " + "configuration_with_properties.configuration == " + "record.configuration?"); + return false; + }, + py::arg("other")) + .def( + "__ne__", + [](config::ConfigurationWithProperties const &, + config::ConfigurationRecord const &) -> bool { + py::module_::import("warnings").attr("warn")( + "Comparing a ConfigurationWithProperties with a " + "ConfigurationRecord directly. Did you mean to compare " + "configuration_with_properties.configuration != " + "record.configuration?"); + return true; + }, + py::arg("other")) .def( "copy", [](config::ConfigurationWithProperties const &self) { diff --git a/python/tests/configuration/test_configuration_set.py b/python/tests/configuration/test_configuration_set.py index 8da709c..13679c9 100644 --- a/python/tests/configuration/test_configuration_set.py +++ b/python/tests/configuration/test_configuration_set.py @@ -146,6 +146,91 @@ def test_ConfigurationSet_to_dict_2(FCC_binary_Hstrain_noshear_prim): assert len(configurations) == 1 +def test_ConfigurationSet_from_dict_duplicate_warning(simple_cubic_binary_prim): + """from_dict warns when the input dict has duplicate configurations.""" + prim = config.Prim(simple_cubic_binary_prim) + supercell = config.make_canonical_supercell( + config.Supercell(prim, np.eye(3, dtype=int)) + ) + configuration = config.Configuration(supercell) + + configurations = config.ConfigurationSet() + configurations.add(configuration) + data = configurations.to_dict() + + # Inject a duplicate entry under a different configuration_id + scel_name = list(data["supercells"].keys())[0] + data["supercells"][scel_name]["1"] = data["supercells"][scel_name]["0"] + + supercells = config.SupercellSet(prim) + with pytest.warns(UserWarning, match="from_dict"): + configurations_in = config.ConfigurationSet.from_dict(data, supercells) + + assert len(configurations_in) == 1 + + +def test_ConfigurationSet_add_record_overwrite_warning(simple_cubic_binary_prim): + """add_record warns and overwrites when the same configuration_name is + inserted with different DoF.""" + prim = config.Prim(simple_cubic_binary_prim) + supercell = config.make_canonical_supercell( + config.Supercell(prim, np.eye(3, dtype=int)) + ) + configuration_a = config.Configuration(supercell) + configuration_b = config.Configuration(supercell) + configuration_b.set_occ(0, 1) + + configurations = config.ConfigurationSet() + record_a = configurations.add(configuration_a) + + # Build a record with the same configuration_name but different DoF + record_b = config.ConfigurationRecord( + configuration_b, + record_a.supercell_name, + record_a.configuration_id, + ) + assert record_a.configuration_name == record_b.configuration_name + assert record_a.configuration != record_b.configuration + + with pytest.warns(UserWarning, match="add_record"): + configurations.add_record(record_b) + + assert len(configurations) == 1 + result = configurations.get(record_a.configuration_name) + assert result is not None + assert result.configuration == configuration_b + + +def test_Configuration_ConfigurationRecord_comparison_warning( + simple_cubic_binary_prim, +): + """Comparing a Configuration with a ConfigurationRecord warns.""" + prim = config.Prim(simple_cubic_binary_prim) + supercell = config.make_canonical_supercell( + config.Supercell(prim, np.eye(3, dtype=int)) + ) + configuration = config.Configuration(supercell) + + configurations = config.ConfigurationSet() + record = configurations.add(configuration) + + with pytest.warns(UserWarning, match="ConfigurationRecord"): + result = configuration == record + assert result is False + + with pytest.warns(UserWarning, match="ConfigurationRecord"): + result = record == configuration + assert result is False + + with pytest.warns(UserWarning, match="ConfigurationRecord"): + result = configuration != record + assert result is True + + with pytest.warns(UserWarning, match="ConfigurationRecord"): + result = record != configuration + assert result is True + + def test_ConfigurationRecord_repr(simple_cubic_binary_prim): prim = config.Prim(simple_cubic_binary_prim) configurations = config.ConfigurationSet() diff --git a/python/tests/configuration/test_configuration_with_properties.py b/python/tests/configuration/test_configuration_with_properties.py index 66859d3..8840866 100644 --- a/python/tests/configuration/test_configuration_with_properties.py +++ b/python/tests/configuration/test_configuration_with_properties.py @@ -1,4 +1,5 @@ import numpy as np +import pytest import libcasm.configuration as casmconfig import libcasm.configuration.io.spglib as spglib_io @@ -419,3 +420,66 @@ def test_configuration_with_properties_io(FCC_binary_prim): assert "configuration" in out assert "global_properties" in out assert "local_properties" in out + + +def test_ConfigurationRecord_ConfigurationWithProperties_comparison_warning( + FCC_binary_prim, +): + """Comparing a ConfigurationWithProperties with a ConfigurationRecord warns.""" + prim = casmconfig.Prim(FCC_binary_prim) + supercell = casmconfig.make_canonical_supercell( + casmconfig.Supercell(prim, np.eye(3, dtype=int)) + ) + configuration = casmconfig.Configuration(supercell) + configuration_with_properties = casmconfig.ConfigurationWithProperties( + configuration + ) + + configurations = casmconfig.ConfigurationSet() + record = configurations.add(configuration) + + with pytest.warns(UserWarning, match="ConfigurationRecord"): + result = configuration_with_properties == record + assert result is False + + with pytest.warns(UserWarning, match="ConfigurationRecord"): + result = record == configuration_with_properties + assert result is False + + with pytest.warns(UserWarning, match="ConfigurationRecord"): + result = configuration_with_properties != record + assert result is True + + with pytest.warns(UserWarning, match="ConfigurationRecord"): + result = record != configuration_with_properties + assert result is True + + +def test_Configuration_ConfigurationWithProperties_comparison_warning( + FCC_binary_prim, +): + """Comparing a Configuration with a ConfigurationWithProperties warns.""" + prim = casmconfig.Prim(FCC_binary_prim) + supercell = casmconfig.make_canonical_supercell( + casmconfig.Supercell(prim, np.eye(3, dtype=int)) + ) + configuration = casmconfig.Configuration(supercell) + configuration_with_properties = casmconfig.ConfigurationWithProperties( + configuration + ) + + with pytest.warns(UserWarning, match="ConfigurationWithProperties"): + result = configuration == configuration_with_properties + assert result is False + + with pytest.warns(UserWarning, match="ConfigurationWithProperties"): + result = configuration_with_properties == configuration + assert result is False + + with pytest.warns(UserWarning, match="ConfigurationWithProperties"): + result = configuration != configuration_with_properties + assert result is True + + with pytest.warns(UserWarning, match="ConfigurationWithProperties"): + result = configuration_with_properties != configuration + assert result is True diff --git a/python/tests/configuration/test_supercell_set.py b/python/tests/configuration/test_supercell_set.py index bdf3f82..592eba5 100644 --- a/python/tests/configuration/test_supercell_set.py +++ b/python/tests/configuration/test_supercell_set.py @@ -287,3 +287,30 @@ def test_SupercellRecord_repr(simple_cubic_binary_prim): assert "supercell_name" in out assert "canonical_supercell_name" in out assert "is_canonical" in out + + +def test_Supercell_SupercellRecord_comparison_warning(simple_cubic_binary_prim): + """Comparing a Supercell with a SupercellRecord warns.""" + prim = config.Prim(simple_cubic_binary_prim) + supercell = config.make_canonical_supercell( + config.Supercell(prim, np.eye(3, dtype=int)) + ) + + supercell_set = config.SupercellSet(prim) + record = supercell_set.add(supercell) + + with pytest.warns(UserWarning, match="SupercellRecord"): + result = supercell == record + assert result is False + + with pytest.warns(UserWarning, match="SupercellRecord"): + result = record == supercell + assert result is False + + with pytest.warns(UserWarning, match="SupercellRecord"): + result = supercell != record + assert result is True + + with pytest.warns(UserWarning, match="SupercellRecord"): + result = record != supercell + assert result is True