diff --git a/doc/source/changelog.rst b/doc/source/changelog.rst index 89e89561..ce5bdbe5 100644 --- a/doc/source/changelog.rst +++ b/doc/source/changelog.rst @@ -10,7 +10,11 @@ MontePy Changelog **Performance Improvement** -* Fixed method of linking ``Material`` to ``ThermalScattering`` objects, avoiding a very expensive O(N:sup:`2`) (:issue:`510`). +* Fixed method of linking ``Material`` to ``ThermalScattering`` objects, avoiding a very expensive O(N :sup:`2`) (:issue:`510`). + +**Deprecations** + +* Marked ``Material.material_components`` as deprecated, and created migration plan describing what to expect moving forward (:issue:`506`). 0.4.0 -------------- diff --git a/doc/source/migrations/migrate0_1.rst b/doc/source/migrations/migrate0_1.rst new file mode 100644 index 00000000..945fd2d6 --- /dev/null +++ b/doc/source/migrations/migrate0_1.rst @@ -0,0 +1,54 @@ +.. _migrate 0 1: + +Migration plan for MontePy 1.0.0 +================================ + +.. meta:: + :description: Migration plan for moving from MontePy 0.x to MontePy 1.0.0 + +Necessity +--------- + +The MCNP 6.2 and 6.3 manuals are ambiguously worded around whether nuclides can be repeated in a material definition. +Due to this the authors of MontePy in MontePy 0.x.x assumed that repeated nuclides were not allowed. +Due to this assumption material composition data were stored in python dictionary, +where the keys were the nuclide and their library. +Due to this if duplicate nuclides are present only the last instance of that nuclide will have its information preserved in the model. +This is clearly not a desired outcome. + +However, it has been confirmed that duplicate nuclides are allowed in MCNP, +and can be advantageous. +See :issue:`504` for more details. +Due to this it was decided that the best way forward was to abandon the old design, +and to create a brand new data structure. +This means that backwards compatibility *will* be broken, +and so this fix is leading to a major version release. + + +Deprecations +------------ +The following properties and objects are currently deprecated, +and will be removed in MontePy 1.0.0. + +* :func:`montepy.data_inputs.material.Material.material_components`. + This is the dictionary that caused this design problem. + +* :class:`montepy.data_inputs.material_components.MaterialComponents` + This is the class that stores information in the above dictionary. + It is largely excess object wrapping, that makes the material interface + overly complex. + +* :class:`montepy.data_inputs.Isotope` will be renamed to ``Nuclide``. + This is to better align with MCNP documentation, + and better reflect that the nuclear data for a nuclide can represent + isotopic, isomeric, or atomic data. + + +New Interface +------------- +Currently the replacement interface has not been fully designed yet. +If you have input you can `join the discussion `_. +There will also be some alpha-testing announced in that discussion. + +Once MontePy 1.0.0 is released this will be updated with information about the new interface, +and how to migrate to it. diff --git a/doc/source/users.rst b/doc/source/users.rst index 7a572473..0e18411d 100644 --- a/doc/source/users.rst +++ b/doc/source/users.rst @@ -9,5 +9,6 @@ User Guide utilities tricks faq + migrations/migrate0_1 changelog publications diff --git a/montepy/data_inputs/isotope.py b/montepy/data_inputs/isotope.py index 81a3d0a3..a0f5a78c 100644 --- a/montepy/data_inputs/isotope.py +++ b/montepy/data_inputs/isotope.py @@ -3,13 +3,21 @@ from montepy.errors import * from montepy.input_parser.syntax_node import PaddingNode, ValueNode +import warnings + class Isotope: """ A class to represent an MCNP isotope + .. deprecated:: 0.4.1 + This will class is deprecated, and will be renamed: ``Nuclde``. + For more details see the :ref:`migrate 0 1`. + :param ZAID: the MCNP isotope identifier :type ZAID: str + :param suppress_warning: Whether to suppress the ``FutureWarning``. + :type suppress_warning: bool """ # Cl-52 Br-101 Xe-150 Os-203 Cm-251 Og-296 @@ -22,7 +30,14 @@ class Isotope: Points on bounding curve for determining if "valid" isotope """ - def __init__(self, ZAID="", node=None): + def __init__(self, ZAID="", node=None, suppress_warning=False): + if not suppress_warning: + warnings.warn( + "montepy.data_inputs.isotope.Isotope is deprecated and will be renamed: Nuclide.\n" + "See for more information ", + FutureWarning, + ) + if node is not None and isinstance(node, ValueNode): if node.type == float: node = ValueNode(node.token, str, node.padding) diff --git a/montepy/data_inputs/material.py b/montepy/data_inputs/material.py index 0c754e95..fab0dbfa 100644 --- a/montepy/data_inputs/material.py +++ b/montepy/data_inputs/material.py @@ -12,6 +12,8 @@ import itertools import re +import warnings + def _number_validator(self, number): if number <= 0: @@ -51,7 +53,7 @@ def __init__(self, input=None): f"Material definitions for material: {self.number} is not valid.", ) for isotope_node, fraction in iterator: - isotope = Isotope(node=isotope_node) + isotope = Isotope(node=isotope_node, suppress_warning=True) fraction.is_negatable_float = True if not set_atom_frac: set_atom_frac = True @@ -68,8 +70,9 @@ def __init__(self, input=None): input, f"Material definitions for material: {self.number} cannot use atom and mass fraction at the same time", ) + self._material_components[isotope] = MaterialComponent( - isotope, fraction + isotope, fraction, suppress_warning=True ) @make_prop_val_node("_old_number") @@ -102,13 +105,23 @@ def is_atom_fraction(self): @property def material_components(self): """ - The internal dictionary containing all the components of this material. + The internal dictionary containing all the components of this material. - The keys are :class:`~montepy.data_inputs.isotope.Isotope` instances, and the values are - :class:`~montepy.data_inputs.material_component.MaterialComponent` instances. + .. deprecated:: 0.4.1 + MaterialComponent has been deprecated as part of a redesign for the material + interface due to a critical bug in how MontePy handles duplicate nuclides. + See :ref:`migrate 0 1`. - :rtype: dict + The keys are :class:`~montepy.data_inputs.isotope.Isotope` instances, and the values are + :class:`~montepy.data_inputs.material_component.MaterialComponent` instances. + + :rtype: dict """ + warnings.warn( + f"""material_components is deprecated, and will be removed in MontePy 1.0.0. +See for more information """, + DeprecationWarning, + ) return self._material_components @make_prop_pointer("_thermal_scattering", thermal_scattering.ThermalScatteringLaw) @@ -149,7 +162,7 @@ def format_for_mcnp_input(self, mcnp_version): def _update_values(self): new_list = syntax_node.IsotopesNode("new isotope list") - for isotope, component in self.material_components.items(): + for isotope, component in self._material_components.items(): isotope._tree.value = isotope.mcnp_str() new_list.append(("_", isotope._tree, component._tree)) self._tree.nodes["data"] = new_list @@ -198,8 +211,8 @@ def __repr__(self): else: ret += "mass\n" - for component in self.material_components: - ret += repr(self.material_components[component]) + "\n" + for component in self._material_components: + ret += repr(self._material_components[component]) + "\n" if self.thermal_scattering: ret += f"Thermal Scattering: {self.thermal_scattering}" @@ -212,7 +225,7 @@ def __str__(self): def _get_material_elements(self): sortable_components = [ (iso, component.fraction) - for iso, component in self.material_components.items() + for iso, component in self._material_components.items() ] sorted_comps = sorted(sortable_components) elements_set = set() @@ -224,7 +237,7 @@ def _get_material_elements(self): return elements def validate(self): - if len(self.material_components) == 0: + if len(self._material_components) == 0: raise IllegalState( f"Material: {self.number} does not have any components defined." ) @@ -237,10 +250,10 @@ def __hash__(self): """ temp_hash = "" - sorted_isotopes = sorted(list(self.material_components.keys())) + sorted_isotopes = sorted(list(self._material_components.keys())) for isotope in sorted_isotopes: temp_hash = hash( - (temp_hash, str(isotope), self.material_components[isotope].fraction) + (temp_hash, str(isotope), self._material_components[isotope].fraction) ) return hash((temp_hash, self.number)) diff --git a/montepy/data_inputs/material_component.py b/montepy/data_inputs/material_component.py index fcb4bd49..e8738575 100644 --- a/montepy/data_inputs/material_component.py +++ b/montepy/data_inputs/material_component.py @@ -3,6 +3,8 @@ from montepy.input_parser.syntax_node import PaddingNode, ValueNode from montepy.utilities import make_prop_val_node +import warnings + def _enforce_positive(self, val): if val <= 0: @@ -15,13 +17,26 @@ class MaterialComponent: For example: this may be H-1 in water: like 1001.80c — 0.6667 + .. deprecated:: 0.4.1 + MaterialComponent has been deprecated as part of a redesign for the material + interface due to a critical bug in how MontePy handles duplicate nuclides. + See :ref:`migrate 0 1`. + :param isotope: the Isotope object representing this isotope :type isotope: Isotope :param fraction: the fraction of this component in the material :type fraction: ValueNode + :param suppress_warning: Whether to suppress the ``DeprecationWarning``. + :type suppress_warning: bool """ - def __init__(self, isotope, fraction): + def __init__(self, isotope, fraction, suppress_warning=False): + if not suppress_warning: + warnings.warn( + f"""MaterialComponent is deprecated, and will be removed in MontePy 1.0.0. +See for more information """, + DeprecationWarning, + ) if not isinstance(isotope, Isotope): raise TypeError(f"Isotope must be an Isotope. {isotope} given") if isinstance(fraction, (float, int)): diff --git a/tests/test_material.py b/tests/test_material.py index 603b92c7..b4a6de76 100644 --- a/tests/test_material.py +++ b/tests/test_material.py @@ -87,11 +87,16 @@ def test_material_format_mcnp(): ) def test_material_comp_init(isotope, conc, error): with pytest.raises(error): - MaterialComponent(Isotope(isotope), conc) + MaterialComponent(Isotope(isotope, suppress_warning=True), conc, True) + + +def test_mat_comp_init_warn(): + with pytest.warns(DeprecationWarning): + MaterialComponent(Isotope("1001.80c", suppress_warning=True), 0.1) def test_material_comp_fraction_setter(): - comp = MaterialComponent(Isotope("1001.80c"), 0.1) + comp = MaterialComponent(Isotope("1001.80c", suppress_warning=True), 0.1, True) comp.fraction = 5.0 assert comp.fraction == pytest.approx(5.0) with pytest.raises(ValueError): @@ -101,7 +106,7 @@ def test_material_comp_fraction_setter(): def test_material_comp_fraction_str(): - comp = MaterialComponent(Isotope("1001.80c"), 0.1) + comp = MaterialComponent(Isotope("1001.80c", suppress_warning=True), 0.1, True) str(comp) repr(comp) @@ -115,23 +120,24 @@ def test_material_update_format(): print(material.format_for_mcnp_input((6, 2, 0))) assert "8016" in material.format_for_mcnp_input((6, 2, 0))[0] # addition - isotope = Isotope("2004.80c") - material.material_components[isotope] = MaterialComponent(isotope, 0.1) - print(material.format_for_mcnp_input((6, 2, 0))) - assert "2004" in material.format_for_mcnp_input((6, 2, 0))[0] - # update - isotope = list(material.material_components.keys())[-1] - print(material.material_components.keys()) - material.material_components[isotope].fraction = 0.7 - print(material.format_for_mcnp_input((6, 2, 0))) - assert "0.7" in material.format_for_mcnp_input((6, 2, 0))[0] - material.material_components[isotope] = MaterialComponent(isotope, 0.6) - print(material.format_for_mcnp_input((6, 2, 0))) - assert "0.6" in material.format_for_mcnp_input((6, 2, 0))[0] - # delete - del material.material_components[isotope] - print(material.format_for_mcnp_input((6, 2, 0))) - assert "8016" in material.format_for_mcnp_input((6, 2, 0))[0] + isotope = Isotope("2004.80c", suppress_warning=True) + with pytest.deprecated_call(): + material.material_components[isotope] = MaterialComponent(isotope, 0.1, True) + print(material.format_for_mcnp_input((6, 2, 0))) + assert "2004" in material.format_for_mcnp_input((6, 2, 0))[0] + # update + isotope = list(material.material_components.keys())[-1] + print(material.material_components.keys()) + material.material_components[isotope].fraction = 0.7 + print(material.format_for_mcnp_input((6, 2, 0))) + assert "0.7" in material.format_for_mcnp_input((6, 2, 0))[0] + material.material_components[isotope] = MaterialComponent(isotope, 0.6, True) + print(material.format_for_mcnp_input((6, 2, 0))) + assert "0.6" in material.format_for_mcnp_input((6, 2, 0))[0] + # delete + del material.material_components[isotope] + print(material.format_for_mcnp_input((6, 2, 0))) + assert "8016" in material.format_for_mcnp_input((6, 2, 0))[0] @pytest.mark.parametrize( @@ -157,8 +163,9 @@ def test_material_init(line, mat_number, is_atom, fractions): assert material.number == mat_number assert material.old_number == mat_number assert material.is_atom_fraction == is_atom - for component, gold in zip(material.material_components.values(), fractions): - assert component.fraction == pytest.approx(gold) + with pytest.deprecated_call(): + for component, gold in zip(material.material_components.values(), fractions): + assert component.fraction == pytest.approx(gold) if "gas" in line: assert material.parameters["gas"]["data"][0].value == pytest.approx(1.0) @@ -175,28 +182,29 @@ def test_bad_init(line): class TestIsotope(TestCase): def test_isotope_init(self): - isotope = Isotope("1001.80c") + with pytest.warns(FutureWarning): + isotope = Isotope("1001.80c") self.assertEqual(isotope.ZAID, "1001") self.assertEqual(isotope.Z, 1) self.assertEqual(isotope.A, 1) self.assertEqual(isotope.element.Z, 1) self.assertEqual(isotope.library, "80c") with self.assertRaises(ValueError): - Isotope("1001.80c.5") + Isotope("1001.80c.5", suppress_warning=True) with self.assertRaises(ValueError): - Isotope("hi.80c") + Isotope("hi.80c", suppress_warning=True) def test_isotope_metastable_init(self): - isotope = Isotope("13426.02c") + isotope = Isotope("13426.02c", suppress_warning=True) self.assertEqual(isotope.ZAID, "13426") self.assertEqual(isotope.Z, 13) self.assertEqual(isotope.A, 26) self.assertTrue(isotope.is_metastable) self.assertEqual(isotope.meta_state, 1) - isotope = Isotope("92635.02c") + isotope = Isotope("92635.02c", suppress_warning=True) self.assertEqual(isotope.A, 235) self.assertEqual(isotope.meta_state, 1) - isotope = Isotope("92935.02c") + isotope = Isotope("92935.02c", suppress_warning=True) self.assertEqual(isotope.A, 235) self.assertEqual(isotope.meta_state, 4) self.assertEqual(isotope.mcnp_str(), "92935.02c") @@ -208,45 +216,45 @@ def test_isotope_metastable_init(self): ("77764", 77, 164, 3), ] for ZA, Z_ans, A_ans, isomer_ans in edge_cases: - isotope = Isotope(ZA + ".80c") + isotope = Isotope(ZA + ".80c", suppress_warning=True) self.assertEqual(isotope.Z, Z_ans) self.assertEqual(isotope.A, A_ans) self.assertEqual(isotope.meta_state, isomer_ans) with self.assertRaises(ValueError): - isotope = Isotope("13826.02c") + isotope = Isotope("13826.02c", suppress_warning=True) def test_isotope_get_base_zaid(self): - isotope = Isotope("92635.02c") + isotope = Isotope("92635.02c", suppress_warning=True) self.assertEqual(isotope.get_base_zaid(), 92235) def test_isotope_library_setter(self): - isotope = Isotope("1001.80c") + isotope = Isotope("1001.80c", suppress_warning=True) isotope.library = "70c" self.assertEqual(isotope.library, "70c") with self.assertRaises(TypeError): isotope.library = 1 def test_isotope_str(self): - isotope = Isotope("1001.80c") + isotope = Isotope("1001.80c", suppress_warning=True) assert isotope.mcnp_str() == "1001.80c" assert isotope.nuclide_str() == "H-1.80c" assert repr(isotope) == "Isotope('H-1.80c')" assert str(isotope) == " H-1 (80c)" - isotope = Isotope("94239.80c") + isotope = Isotope("94239.80c", suppress_warning=True) assert isotope.nuclide_str() == "Pu-239.80c" assert isotope.mcnp_str() == "94239.80c" assert repr(isotope) == "Isotope('Pu-239.80c')" - isotope = Isotope("92635.80c") + isotope = Isotope("92635.80c", suppress_warning=True) assert isotope.nuclide_str() == "U-235m1.80c" assert isotope.mcnp_str() == "92635.80c" assert str(isotope) == " U-235m1 (80c)" assert repr(isotope) == "Isotope('U-235m1.80c')" # stupid legacy stupidity #486 - isotope = Isotope("95642") + isotope = Isotope("95642", suppress_warning=True) assert isotope.nuclide_str() == "Am-242" assert isotope.mcnp_str() == "95642" assert repr(isotope) == "Isotope('Am-242')" - isotope = Isotope("95242") + isotope = Isotope("95242", suppress_warning=True) assert isotope.nuclide_str() == "Am-242m1" assert isotope.mcnp_str() == "95242" assert repr(isotope) == "Isotope('Am-242m1')"