JDFTXInfile addition (__add__) method tweak (#4407)

* Changing addition method - now infiles with shared keys will either concatenate their inputs if the key is for a repeatable tag, or change to whatever value is in the second infile if it is not a repeatable tag. A bare minimum `if subval in params[key]` check is done to avoid adding duplicate values. This seems like something the `set` built-in could help with, but since the sub-values are dictionaries, using `set` is a little more difficult

* Updated todos

Author: benrich37

src/pymatgen/io/jdftx/inputs.py

@@ -46,6 +46,10 @@ class is written.
 
 # TODO: Add check for whether all ions have or lack velocities.
 # TODO: Add default value filling like JDFTx does.
+# TODO: Add more robust checking for if two repeatable tag values represent the
+# same information. This is likely fixed by implementing filling of default values.
+# TODO: Incorporate something to collapse repeated dump tags of the same frequency
+# into a single value.
 
 
 class JDFTXInfile(dict, MSONable):
@@ -80,18 +84,34 @@ def __add__(self, other: JDFTXInfile) -> JDFTXInfile:
         """Add existing JDFTXInfile object to method caller JDFTXInfile object.
 
         Add all the values of another JDFTXInfile object to this object. Facilitate the use of "standard" JDFTXInfiles.
+        Repeatable tags are appended together. Non-repeatable tags are replaced with their value from the other object.
 
         Args:
             other (JDFTXInfile): JDFTXInfile object to add to the method caller object.
 
         Returns:
             JDFTXInfile: The combined JDFTXInfile object.
         """
-        params: dict[str, Any] = dict(self.items())
+        # Deepcopy needed here, or else in `jif1 = jif2 + jif3`, `jif1` will become a reference to `jif2`
+        params: dict[str, Any] = deepcopy(dict(self.items()))
         for key, val in other.items():
-            if key in self and val != self[key]:
-                raise ValueError(f"JDFTXInfiles have conflicting values for {key}: {self[key]} != {val}")
-            params[key] = val
+            if key in self:
+                if val is params[key]:
+                    # Unlinking the two objects fully cannot be done by deepcopy for some reason
+                    continue
+                tag_object = get_tag_object(key)
+                if tag_object.can_repeat:
+                    if isinstance(val, list):
+                        for subval in list(val):
+                            # Minimum effort to avoid duplicates
+                            if subval not in params[key]:
+                                params[key].append(subval)
+                    else:
+                        params[key].append(val)
+                else:
+                    params[key] = val
+            else:
+                params[key] = val
         return type(self)(params)
 
     def as_dict(self, sort_tags: bool = True, skip_module_keys: bool = False) -> dict:

tests/io/jdftx/test_jdftxinfile.py

@@ -214,6 +214,7 @@ def test_JDFTXInfile_add_method():
     # repeatable values, repeatable tags are not append to each other
     jif = JDFTXInfile.from_file(ex_infile1_fname)
     jif2 = jif.copy()
+    assert jif2 is not jif  # Testing robustness of copy method while we are at it
     jif3 = jif + jif2
     assert_idential_jif(jif, jif3)
     # If a tag is repeated, the values must be the same since choice of value is ambiguous
@@ -222,9 +223,17 @@ def test_JDFTXInfile_add_method():
     val_new = "lda"
     assert val_old != val_new
     jif2[key] = val_new
-    err_str = f"JDFTXInfiles have conflicting values for {key}: {val_old} != {val_new}"
-    with pytest.raises(ValueError, match=re.escape(err_str)):
-        jif3 = jif + jif2
+    jif4 = jif + jif2
+    assert_same_value(jif4[key], val_new)  # Make sure addition chooses second value for non-repeatable tags
+    del jif4
+    jif2.append_tag("dump", "Fluid State")
+    jif.append_tag("dump", "Fluid Berry")
+    jif4 = jif + jif2
+    assert len(jif4["dump"]) == len(jif2["dump"]) + 1
+    assert {"Fluid": {"State": True}} in jif4["dump"]
+    assert {"Fluid": {"State": True}} not in jif["dump"]
+    assert {"Fluid": {"Berry": True}} in jif4["dump"]
+    assert {"Fluid": {"Berry": True}} not in jif2["dump"]
     # Normal expected behavior
     key_add = "target-mu"
     val_add = 0.5