Bug fix: Hydrogen and organic elements were ignored by the `forbidden…

…_elements` (#31)

* add one-pager overview

Signed-off-by: Marcel Müller <marcel.mueller@thch.uni-bonn.de>

* fix edge case that HCNO is in forbidden elements

Signed-off-by: Marcel Müller <marcel.mueller@thch.uni-bonn.de>

* update CHANGELOG.md

Signed-off-by: Marcel Müller <marcel.mueller@thch.uni-bonn.de>

---------

Signed-off-by: Marcel Müller <marcel.mueller@thch.uni-bonn.de>

CHANGELOG.md

@@ -13,6 +13,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Adapted generation of number of unpaired electrons; thereby, support for Ln's
 - Shifted group / element sorting definitions to miscellaneous
 - `xyz` files are written on the fly, and not post-generation
+- `forbidden_elements` and `element_composition` influences hydrogen and organic element addition
 
 ### Added
 - Optimization via DFT in the post-processing step

src/mindlessgen/generator/main.py

@@ -158,7 +158,19 @@ def single_molecule_generator(
     # | |__| |  __/ | | |  __/ | | (_| | || (_) | |
     #  \_____|\___|_| |_|\___|_|  \__,_|\__\___/|_|
 
-    mol = generate_random_molecule(config.generate, config.general.verbosity)
+    try:
+        mol = generate_random_molecule(config.generate, config.general.verbosity)
+    # RuntimeError is not caught here, as in this part, runtime errors are not expected to occur
+    # and shall therefore be raised to the main function
+    except (
+        SystemExit
+    ) as e:  # debug functionality: raise SystemExit to stop the whole execution
+        if config.general.verbosity > 0:
+            print(f"Generation aborted for cycle {cycle + 1}.")
+            if config.general.verbosity > 1:
+                print(e)
+        stop_event.set()
+        return None
 
     try:
         #    ____        _   _           _

src/mindlessgen/molecules/generate_molecule.py

@@ -173,13 +173,20 @@ def add_random(min_adds: int, max_adds: int, min_nat: int, max_nat: int):
             )  # with range(0, 3) -> mean value: 1
             # max value of this section with commented settings: 12
 
-    def add_organic(num_adds: int, min_nat: int, max_nat: int):
+    def add_organic(num_adds: int, min_nat: int, max_nat: int) -> None:
         """
         Add organic elements.
         """
         # Add Elements between B and F (5-9)
+        valid_organic: list[int] = []
+        for organic_index in range(4, 10):
+            if organic_index in valid_elems:
+                valid_organic.append(organic_index)
+        if not valid_organic:
+            return
         for _ in range(num_adds):  # with range(5) -> mean value 1.5
-            ati = np.random.randint(4, 10)
+            # go through the elements B to F (4-9 in 0-based indexing)
+            ati = np.random.choice(valid_organic)
             if verbosity > 1:
                 print(f"Adding atom type {ati}...")
             natoms[ati] = natoms[ati] + np.random.randint(
@@ -298,9 +305,11 @@ def check_composition():
     # Check for too many transition and lanthanide metals
     remove_metals()
     # Add organic elements (B, C, N, O, F)
-    add_organic(lim_organic, 0, 3)
+    add_organic(num_adds=lim_organic, min_nat=0, max_nat=3)
     # Add hydrogen if not included
-    add_hydrogen()
+    # execute only if hydrogen is included in the valid elements
+    if 0 in valid_elems:
+        add_hydrogen()
     # Check if pre-defined atom type counts are within the defined limits
     check_composition()
     # Check if the number of atoms is within the defined limits

src/mindlessgen/prog/config.py

@@ -291,6 +291,8 @@ def element_composition(self, composition_str):
 
         if not isinstance(composition_str, str):
             raise TypeError("Element composition should be a string.")
+        if not composition_str:
+            return
 
         element_dict = {}
         elements = composition_str.split(",")
@@ -333,6 +335,10 @@ def forbidden_elements(self: GenerateConfig, forbidden_str: str) -> None:
 
         Format: "57-71, 8, 1" or "19-*"
         """
+        # if string is empty or None, set to None
+        if not forbidden_str:
+            self._forbidden_elements = None
+            return
         forbidden_set: set[int] = set()
         elements = forbidden_str.split(",")
         elements = [element.strip() for element in elements]

test/test_generate/test_generate_molecule.py

@@ -284,3 +284,93 @@ def test_generate_atom_list_min_larger_than_max(default_generate_config):
 
     with pytest.raises(ValueError):
         generate_atom_list(default_generate_config, verbosity=1)
+
+
+# Test to ensure non-integer values for min/max atoms raise errors
+@pytest.mark.parametrize(
+    "min_atoms, max_atoms, expected_error",
+    [
+        ("five", 10, TypeError),  # Non-integer min atoms
+        (5, "ten", TypeError),  # Non-integer max atoms
+        (2.5, 15, TypeError),  # Float as min atoms
+        (5, 20.5, TypeError),  # Float as max atoms
+    ],
+)
+def test_invalid_min_max_atoms(
+    min_atoms, max_atoms, expected_error, default_generate_config
+):
+    """Test for non-integer min/max atom values raising TypeErrors."""
+    with pytest.raises(expected_error):
+        default_generate_config.min_num_atoms = min_atoms
+        default_generate_config.max_num_atoms = max_atoms
+
+
+# Edge case where forbidden elements overlap allowed range
+@pytest.mark.parametrize(
+    "forbidden_elements, expected_atoms",
+    [
+        ("1-5", [0, 1, 2, 3, 4]),  # Banned elements within the default organic range
+        ("1, 6, 7", [0, 5, 6]),  # Specific elements banned
+    ],
+)
+def test_generate_atom_list_with_overlapping_forbidden_elements(
+    forbidden_elements, expected_atoms, default_generate_config
+):
+    """Test generate_atom_list when forbidden elements overlap with allowed ranges."""
+    default_generate_config.forbidden_elements = forbidden_elements
+    default_generate_config.min_num_atoms = 5
+    default_generate_config.max_num_atoms = 15
+    atom_list = generate_atom_list(default_generate_config, verbosity=1)
+
+    # Ensure forbidden elements are not present in the atom list
+    assert np.sum([atom_list[z] for z in expected_atoms]) == 0
+
+
+# Test behavior when composition is empty but min/max are set
+def test_generate_atom_list_with_empty_composition(default_generate_config):
+    """Ensure empty compositions don't lead to unexpected behaviors."""
+    default_generate_config.element_composition = ""
+    default_generate_config.min_num_atoms = 5
+    default_generate_config.max_num_atoms = 10
+    atom_list = generate_atom_list(default_generate_config, verbosity=1)
+
+    # Ensure some atoms are still generated within min and max limits
+    assert np.sum(atom_list) >= default_generate_config.min_num_atoms
+    assert np.sum(atom_list) <= default_generate_config.max_num_atoms
+
+
+# Test for element compositions with zero ranges
+def test_generate_atom_list_zero_composition(default_generate_config):
+    """Test generate_atom_list when compositions have zero counts."""
+    default_generate_config.element_composition = "C:0-0, N:0-0, O:0-0"
+    default_generate_config.min_num_atoms = 5
+    default_generate_config.max_num_atoms = 10
+    atom_list = generate_atom_list(default_generate_config, verbosity=1)
+
+    # Ensure atoms in these ranges are indeed zero
+    assert atom_list[5] == 0  # C
+    assert atom_list[6] == 0  # N
+    assert atom_list[7] == 0  # O
+
+
+# Check hydrogen addition when it should/shouldn't occur
+@pytest.mark.parametrize(
+    "forbidden_elements, should_contain_hydrogen",
+    [
+        ("1", False),  # Hydrogen forbidden
+        ("", True),  # No forbidden elements
+    ],
+)
+def test_hydrogen_addition(
+    forbidden_elements, should_contain_hydrogen, default_generate_config
+):
+    """Test hydrogen addition based on forbidden elements."""
+    default_generate_config.forbidden_elements = forbidden_elements
+    default_generate_config.min_num_atoms = 5
+    default_generate_config.max_num_atoms = 15
+    atom_list = generate_atom_list(default_generate_config, verbosity=1)
+
+    if should_contain_hydrogen:
+        assert atom_list[0] > 0
+    else:
+        np.testing.assert_equal(atom_list[0], 0)