Fix in-place VaspInput.incar updates having no effect if incar is…

… dict (not `Incar` instance) (#4052)

* in-place VaspInput.incar updates have no effect if incar is dict (not Incar instance)

* TestVaspInput check incar attr can be updated in place

src/pymatgen/io/vasp/inputs.py

@@ -2743,7 +2743,7 @@ def __init__(
         """
         super().__init__(**kwargs)
         self._potcar_filename = "POTCAR" + (".spec" if potcar_spec else "")
-        self.update({"INCAR": incar, "KPOINTS": kpoints, "POSCAR": poscar, self._potcar_filename: potcar})
+        self.update({"INCAR": Incar(incar), "KPOINTS": kpoints, "POSCAR": poscar, self._potcar_filename: potcar})
         if optional_files is not None:
             self.update(optional_files)
 
@@ -2771,7 +2771,7 @@ def from_dict(cls, dct: dict) -> Self:
         """
         sub_dct: dict[str, dict] = {"optional_files": {}}
         for key, val in dct.items():
-            if key in ["INCAR", "POSCAR", "POTCAR", "KPOINTS"]:
+            if key in ("INCAR", "POSCAR", "POTCAR", "KPOINTS"):
                 sub_dct[key.lower()] = MontyDecoder().process_decoded(val)
             elif key not in ["@module", "@class"]:
                 sub_dct["optional_files"][key] = MontyDecoder().process_decoded(val)
@@ -2907,7 +2907,7 @@ def run_vasp(
     @property
     def incar(self) -> Incar:
         """INCAR object."""
-        return Incar(self["INCAR"]) if isinstance(self["INCAR"], dict) else self["INCAR"]
+        return self["INCAR"]
 
     @property
     def kpoints(self) -> Kpoints | None:

tests/io/vasp/test_inputs.py

@@ -1365,14 +1365,16 @@ def test_copy(self):
         assert vasp_input2.as_dict() == self.vasp_input.as_dict()
         # modify the copy and make sure the original is not modified
         vasp_input2["INCAR"]["NSW"] = 100
-        assert vasp_input2["INCAR"]["NSW"] == 100
-        assert self.vasp_input["INCAR"]["NSW"] == 99
+        assert vasp_input2["INCAR"]["NSW"] == 100, f"{vasp_input2['INCAR']['NSW']=}"
+        orig_nsw_val = self.vasp_input["INCAR"]["NSW"]
+        assert orig_nsw_val == 99, f"{orig_nsw_val=}"
 
         # make a shallow copy and make sure the original is modified
         vasp_input3 = self.vasp_input.copy(deep=False)
         vasp_input3["INCAR"]["NSW"] = 100
-        assert vasp_input3["INCAR"]["NSW"] == 100
-        assert self.vasp_input["INCAR"]["NSW"] == 100
+        assert vasp_input3["INCAR"]["NSW"] == 100, f"{vasp_input3['INCAR']['NSW']=}"
+        orig_nsw_val = self.vasp_input["INCAR"]["NSW"]
+        assert orig_nsw_val == 99, f"{orig_nsw_val=}"
 
     def test_run_vasp(self):
         self.vasp_input.run_vasp(".", vasp_cmd=["cat", "INCAR"])
@@ -1402,6 +1404,7 @@ def test_from_directory(self):
         assert "CONTCAR_Li2O" in vasp_input
 
     def test_input_attr(self):
+        # test attributes match dict keys
         assert all(v == getattr(self.vasp_input, k.lower()) for k, v in self.vasp_input.items())
 
         vis_potcar_spec = VaspInput(
@@ -1411,10 +1414,17 @@ def test_input_attr(self):
             "\n".join(self.vasp_input.potcar.symbols),
             potcar_spec=True,
         )
-        assert all(k in vis_potcar_spec for k in ("INCAR", "KPOINTS", "POSCAR", "POTCAR.spec"))
+        # test has expected keys
+        assert {*vis_potcar_spec} == {"INCAR", "KPOINTS", "POSCAR", "POTCAR.spec"}
+        # test values match
         assert all(self.vasp_input[k] == getattr(vis_potcar_spec, k.lower()) for k in ("INCAR", "KPOINTS", "POSCAR"))
         assert isinstance(vis_potcar_spec.potcar, str)
 
+        # test incar can be updated in place, see https://github.com/materialsproject/pymatgen/issues/4051
+        assert vis_potcar_spec.incar["NSW"] == 99
+        vis_potcar_spec.incar["NSW"] = 100
+        assert vis_potcar_spec.incar["NSW"] == 100
+
 
 def test_potcar_summary_stats() -> None:
     potcar_summary_stats = loadfn(POTCAR_STATS_PATH)