From fb3dce4f8d3b03928e4a47766966f15b1bd27bec Mon Sep 17 00:00:00 2001 From: Yuan Chiang Date: Tue, 16 Apr 2024 13:23:06 -0700 Subject: [PATCH 01/11] bugfix: allow custom chgcar path --- pymatgen/command_line/chargemol_caller.py | 58 ++++++++++++----------- 1 file changed, 31 insertions(+), 27 deletions(-) diff --git a/pymatgen/command_line/chargemol_caller.py b/pymatgen/command_line/chargemol_caller.py index 7f2fc1bcd91..ffbffe9e6a0 100644 --- a/pymatgen/command_line/chargemol_caller.py +++ b/pymatgen/command_line/chargemol_caller.py @@ -46,6 +46,7 @@ import subprocess import warnings from glob import glob +from pathlib import Path from shutil import which from typing import TYPE_CHECKING @@ -57,8 +58,6 @@ from pymatgen.io.vasp.outputs import Chgcar if TYPE_CHECKING: - from pathlib import Path - from pymatgen.core import Structure __author__ = "Martin Siron, Andrew S. Rosen" @@ -96,7 +95,7 @@ def __init__( run_chargemol (bool): Whether to run the Chargemol analysis. If False, the existing Chargemol output files will be read from path. Default: True. """ - path = path or os.getcwd() + path = (Path(path).absolute() if path else None) or os.getcwd() if run_chargemol and not CHARGEMOL_EXE: raise OSError( "ChargemolAnalysis requires the Chargemol executable to be in PATH." @@ -170,34 +169,34 @@ def _execute_chargemol(self, **job_control_kwargs): """Internal function to run Chargemol. Args: + path (str): Path to the directory to run Chargemol in. + Default: None (current working directory). atomic_densities_path (str): Path to the atomic densities directory required by Chargemol. If None, Pymatgen assumes that this is defined in a "DDEC6_ATOMIC_DENSITIES_DIR" environment variable. Default: None. job_control_kwargs: Keyword arguments for _write_jobscript_for_chargemol. """ - with ScratchDir("."): - try: - os.symlink(self._chgcar_path, "./CHGCAR") - os.symlink(self._potcar_path, "./POTCAR") - os.symlink(self._aeccar0_path, "./AECCAR0") - os.symlink(self._aeccar2_path, "./AECCAR2") - except OSError as exc: - print(f"Error creating symbolic link: {exc}") - - # write job_script file: - self._write_jobscript_for_chargemol(**job_control_kwargs) - - # Run Chargemol - with subprocess.Popen(CHARGEMOL_EXE, stdout=subprocess.PIPE, stdin=subprocess.PIPE, close_fds=True) as rs: - _stdout, stderr = rs.communicate() - if rs.returncode != 0: - raise RuntimeError( - f"{CHARGEMOL_EXE} exit code: {rs.returncode}, error message: {stderr!s}. " - "Please check your Chargemol installation." - ) - - self._from_data_dir() + with ScratchDir(".") as temp_dir: + # with tempfile.TemporaryDirectory() as temp_dir: + + os.symlink(self._chgcar_path, "CHGCAR") + os.symlink(self._potcar_path, "POTCAR") + os.symlink(self._aeccar0_path, "AECCAR0") + os.symlink(self._aeccar2_path, "AECCAR2") + + lines = self._write_jobscript_for_chargemol( + chgcar_path=os.path.join(temp_dir, "CHGCAR"), + **job_control_kwargs, + ) + with open(os.path.join(temp_dir, "job_control.txt"), mode="w") as file: + file.write(lines) + + rs = subprocess.run(CHARGEMOL_EXE, capture_output=True, check=False) + _stdout = rs.stdout.decode("utf-8") + rs.stderr.decode("utf-8") + + self._from_data_dir(chargemol_output_path=temp_dir) def _from_data_dir(self, chargemol_output_path=None): """Internal command to parse Chargemol files from a directory. @@ -337,6 +336,7 @@ def get_bond_order(self, index_from, index_to): def _write_jobscript_for_chargemol( self, + chgcar_path=None, net_charge=0.0, periodicity=(True, True, True), method="ddec6", @@ -345,6 +345,8 @@ def _write_jobscript_for_chargemol( """Writes job_script.txt for Chargemol execution. Args: + chgcar_path (str): Path to the CHGCAR file. If None, CHGCAR is assumed to be + in the directory where the job_script.txt is written. net_charge (float): Net charge of the system. Defaults to 0.0. periodicity (tuple[bool]): Periodicity of the system. @@ -363,6 +365,9 @@ def _write_jobscript_for_chargemol( if net_charge: lines += f"\n{net_charge}\n\n" + if chgcar_path: + lines += f"\n{chgcar_path}\n\n" + # Periodicity if periodicity: per_a = ".true." if periodicity[0] else ".false." @@ -402,8 +407,7 @@ def _write_jobscript_for_chargemol( bo = ".true." if compute_bond_orders else ".false." lines += f"\n\n{bo}\n\n" - with open("job_control.txt", mode="w") as file: - file.write(lines) + return lines @staticmethod def _get_dipole_info(filepath): From 637501ad87e3e55aeea5cab2b454d2fc3b9f5a49 Mon Sep 17 00:00:00 2001 From: Yuan Chiang Date: Sat, 20 Apr 2024 16:46:03 -0700 Subject: [PATCH 02/11] atomic_densities is a direcotry, not file --- pymatgen/command_line/chargemol_caller.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pymatgen/command_line/chargemol_caller.py b/pymatgen/command_line/chargemol_caller.py index ffbffe9e6a0..d85674557e2 100644 --- a/pymatgen/command_line/chargemol_caller.py +++ b/pymatgen/command_line/chargemol_caller.py @@ -385,7 +385,7 @@ def _write_jobscript_for_chargemol( "The DDEC6_ATOMIC_DENSITIES_DIR environment variable must be set or the atomic_densities_path must" " be specified" ) - if not os.path.isfile(atomic_densities_path): + if not os.path.exists(atomic_densities_path): raise FileNotFoundError(f"{atomic_densities_path=} does not exist") # This is to fix a Chargemol filepath nuance From ba0f1a926773f47a9533ecb7bf5cad5abe6c3b38 Mon Sep 17 00:00:00 2001 From: Janosh Riebesell Date: Sun, 21 Apr 2024 15:41:24 -0700 Subject: [PATCH 03/11] refactor TestChargemolAnalysis --- tests/command_line/test_chargemol_caller.py | 79 +++++++++++---------- 1 file changed, 42 insertions(+), 37 deletions(-) diff --git a/tests/command_line/test_chargemol_caller.py b/tests/command_line/test_chargemol_caller.py index 8ec8f4a2ee2..1b3202d5248 100644 --- a/tests/command_line/test_chargemol_caller.py +++ b/tests/command_line/test_chargemol_caller.py @@ -1,55 +1,60 @@ from __future__ import annotations from pymatgen.command_line.chargemol_caller import ChargemolAnalysis -from pymatgen.core import Element +from pymatgen.core import Element, Structure from pymatgen.util.testing import TEST_FILES_DIR class TestChargemolAnalysis: def test_parse_chargemol(self): test_dir = f"{TEST_FILES_DIR}/chargemol/spin_unpolarized" - ca = ChargemolAnalysis(path=test_dir, run_chargemol=False) - assert ca.ddec_charges == [0.8432, -0.8432] - assert ca.get_partial_charge(0) == 0.8432 - assert ca.get_partial_charge(0, charge_type="cm5") == 0.420172 - assert ca.get_charge_transfer(0) == -0.8432 - assert ca.get_charge_transfer(0, charge_type="cm5") == -0.420172 - assert ca.get_charge(0, nelect=1) == 1 - 0.8432 - assert ca.get_charge(0, nelect=1, charge_type="cm5") == 1 - 0.420172 - assert ca.dipoles == [[0.0, 0.0, 0.0], [0.0, 0.0, 0.0]] - assert ca.ddec_spin_moments is None - assert ca.bond_order_sums == [0.53992, 0.901058] - assert ca.ddec_rsquared_moments == [8.261378, 34.237274] - assert ca.ddec_rcubed_moments == [14.496002, 88.169236] - assert ca.ddec_rfourth_moments == [37.648248, 277.371929] - assert ca.cm5_charges == [0.420172, -0.420172] - assert ca.summary["ddec"]["partial_charges"] == ca.ddec_charges - assert ca.summary["ddec"]["dipoles"] == ca.dipoles - assert ca.summary["ddec"]["bond_order_sums"] == ca.bond_order_sums - assert ca.summary["ddec"]["rsquared_moments"] == ca.ddec_rsquared_moments - assert ca.summary["ddec"]["rcubed_moments"] == ca.ddec_rcubed_moments - assert ca.summary["ddec"]["rfourth_moments"] == ca.ddec_rfourth_moments - assert ca.summary["cm5"]["partial_charges"] == ca.cm5_charges - assert ca.summary["ddec"]["bond_order_dict"] == ca.bond_order_dict - assert ca.summary["ddec"].get("spin_moments") is None - assert ca.natoms == [1, 1] - assert ca.structure is not None - assert len(ca.bond_order_dict) == 2 - assert ca.bond_order_dict[0]["bonded_to"][0] == { + chg_mol = ChargemolAnalysis(path=test_dir, run_chargemol=False) + assert chg_mol.ddec_charges == [0.8432, -0.8432] + assert chg_mol.get_partial_charge(0) == 0.8432 + assert chg_mol.get_partial_charge(0, charge_type="cm5") == 0.420172 + assert chg_mol.get_charge_transfer(0) == -0.8432 + assert chg_mol.get_charge_transfer(0, charge_type="cm5") == -0.420172 + assert chg_mol.get_charge(0, nelect=1) == 1 - 0.8432 + assert chg_mol.get_charge(0, nelect=1, charge_type="cm5") == 1 - 0.420172 + assert chg_mol.dipoles == [[0.0, 0.0, 0.0], [0.0, 0.0, 0.0]] + assert chg_mol.ddec_spin_moments is None + assert chg_mol.bond_order_sums == [0.53992, 0.901058] + assert chg_mol.ddec_rsquared_moments == [8.261378, 34.237274] + assert chg_mol.ddec_rcubed_moments == [14.496002, 88.169236] + assert chg_mol.ddec_rfourth_moments == [37.648248, 277.371929] + assert chg_mol.cm5_charges == [0.420172, -0.420172] + assert chg_mol.summary["ddec"]["partial_charges"] == chg_mol.ddec_charges + assert chg_mol.summary["ddec"]["dipoles"] == chg_mol.dipoles + assert chg_mol.summary["ddec"]["bond_order_sums"] == chg_mol.bond_order_sums + assert chg_mol.summary["ddec"]["rsquared_moments"] == chg_mol.ddec_rsquared_moments + assert chg_mol.summary["ddec"]["rcubed_moments"] == chg_mol.ddec_rcubed_moments + assert chg_mol.summary["ddec"]["rfourth_moments"] == chg_mol.ddec_rfourth_moments + assert chg_mol.summary["cm5"]["partial_charges"] == chg_mol.cm5_charges + assert chg_mol.summary["ddec"]["bond_order_dict"] == chg_mol.bond_order_dict + assert chg_mol.summary["ddec"].get("spin_moments") is None + assert chg_mol.natoms == [1, 1] + assert isinstance(chg_mol.structure, Structure) + assert len(chg_mol.structure) == 2 + assert chg_mol.structure.formula == "Na1 Cl1" + assert len(chg_mol.bond_order_dict) == 2 + assert chg_mol.bond_order_dict[0]["bonded_to"][0] == { "spin_polarization": 0.0, "index": 1, "direction": (-1, -1, 0), "bond_order": 0.0882, "element": Element("Cl"), } - assert ca.bond_order_dict[1]["bonded_to"][-1]["direction"] == (-1, 0, 0) - assert ca.get_property_decorated_structure().site_properties["partial_charge_ddec6"] == ca.ddec_charges + assert chg_mol.bond_order_dict[1]["bonded_to"][-1]["direction"] == (-1, 0, 0) + # check that partial charges are written to structure site properties + charge_decorated_struct = chg_mol.get_property_decorated_structure() + struct_partial_charge = charge_decorated_struct.site_properties["partial_charge_ddec6"] + assert struct_partial_charge == chg_mol.ddec_charges def test_parse_chargemol2(self): test_dir = f"{TEST_FILES_DIR}/chargemol/spin_polarized" - ca = ChargemolAnalysis(path=test_dir, run_chargemol=False) - assert ca.ddec_spin_moments == [0.201595, 0.399203, 0.399203] - assert ca.summary["ddec"]["bond_order_dict"][0]["bonded_to"][0]["spin_polarization"] == 0.0490 - assert ca.summary["ddec"]["spin_moments"] == ca.ddec_spin_moments - assert ca.natoms is None - assert ca.structure is None + chg_mol = ChargemolAnalysis(path=test_dir, run_chargemol=False) + assert chg_mol.ddec_spin_moments == [0.201595, 0.399203, 0.399203] + assert chg_mol.summary["ddec"]["bond_order_dict"][0]["bonded_to"][0]["spin_polarization"] == 0.0490 + assert chg_mol.summary["ddec"]["spin_moments"] == chg_mol.ddec_spin_moments + assert chg_mol.natoms is None + assert chg_mol.structure is None From 7a290264717c1f1a0bfe465e58e3d4fcdf87c4bb Mon Sep 17 00:00:00 2001 From: Janosh Riebesell Date: Sun, 21 Apr 2024 15:42:19 -0700 Subject: [PATCH 04/11] prefer os.path over pathlib --- pymatgen/command_line/chargemol_caller.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/pymatgen/command_line/chargemol_caller.py b/pymatgen/command_line/chargemol_caller.py index d85674557e2..7c9b61bf5ac 100644 --- a/pymatgen/command_line/chargemol_caller.py +++ b/pymatgen/command_line/chargemol_caller.py @@ -46,7 +46,6 @@ import subprocess import warnings from glob import glob -from pathlib import Path from shutil import which from typing import TYPE_CHECKING @@ -58,6 +57,8 @@ from pymatgen.io.vasp.outputs import Chgcar if TYPE_CHECKING: + from pathlib import Path + from pymatgen.core import Structure __author__ = "Martin Siron, Andrew S. Rosen" @@ -95,7 +96,7 @@ def __init__( run_chargemol (bool): Whether to run the Chargemol analysis. If False, the existing Chargemol output files will be read from path. Default: True. """ - path = (Path(path).absolute() if path else None) or os.getcwd() + path = os.path.abspath(path) if path else os.getcwd() if run_chargemol and not CHARGEMOL_EXE: raise OSError( "ChargemolAnalysis requires the Chargemol executable to be in PATH." From 76324b38bb2c44b72ddcb6bfd02c59040a9e10c8 Mon Sep 17 00:00:00 2001 From: Janosh Riebesell Date: Sun, 21 Apr 2024 15:43:57 -0700 Subject: [PATCH 05/11] fix default atomic_densities_path (set to None, so if == "" doesn't apply) --- pymatgen/command_line/chargemol_caller.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/pymatgen/command_line/chargemol_caller.py b/pymatgen/command_line/chargemol_caller.py index 7c9b61bf5ac..bf250a3e43b 100644 --- a/pymatgen/command_line/chargemol_caller.py +++ b/pymatgen/command_line/chargemol_caller.py @@ -103,9 +103,7 @@ def __init__( " Please download the library at https://sourceforge.net/projects/ddec/files" "and follow the instructions." ) - if atomic_densities_path == "": - atomic_densities_path = os.getcwd() - self._atomic_densities_path = atomic_densities_path + self._atomic_densities_path = atomic_densities_path or os.getcwd() self._chgcar_path = self._get_filepath(path, "CHGCAR") self._potcar_path = self._get_filepath(path, "POTCAR") From 1832070ee44ae00715da785ec91fd9eebc8d769d Mon Sep 17 00:00:00 2001 From: Janosh Riebesell Date: Sun, 21 Apr 2024 15:44:55 -0700 Subject: [PATCH 06/11] fix possibly unbound var in get_charge_transfer --- pymatgen/command_line/chargemol_caller.py | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/pymatgen/command_line/chargemol_caller.py b/pymatgen/command_line/chargemol_caller.py index bf250a3e43b..38e9ec96975 100644 --- a/pymatgen/command_line/chargemol_caller.py +++ b/pymatgen/command_line/chargemol_caller.py @@ -47,7 +47,7 @@ import warnings from glob import glob from shutil import which -from typing import TYPE_CHECKING +from typing import TYPE_CHECKING, Literal import numpy as np from monty.tempfile import ScratchDir @@ -249,7 +249,7 @@ def _from_data_dir(self, chargemol_output_path=None): else: self.cm5_charges = None - def get_charge_transfer(self, atom_index, charge_type="ddec"): + def get_charge_transfer(self, atom_index: int, charge_type: Literal["cm5", "ddec"] = "ddec") -> float: """Returns the charge transferred for a particular atom. A positive value means that the site has gained electron density (i.e. exhibits anionic character) whereas a negative value means the site has lost electron density (i.e. exhibits @@ -263,13 +263,11 @@ def get_charge_transfer(self, atom_index, charge_type="ddec"): Returns: float: charge transferred at atom_index """ - if charge_type.lower() not in ["ddec", "cm5"]: - raise ValueError(f"Invalid {charge_type=}") if charge_type.lower() == "ddec": - charge_transfer = -self.ddec_charges[atom_index] - elif charge_type.lower() == "cm5": - charge_transfer = -self.cm5_charges[atom_index] - return charge_transfer + return -self.ddec_charges[atom_index] + if charge_type.lower() == "cm5": + return -self.cm5_charges[atom_index] + raise ValueError(f"Invalid {charge_type=}") def get_charge(self, atom_index, nelect=None, charge_type="ddec"): """Convenience method to get the charge on a particular atom using the same From a05635ee0b856acd629b9e317335599879b3f31d Mon Sep 17 00:00:00 2001 From: Janosh Riebesell Date: Sun, 21 Apr 2024 15:48:54 -0700 Subject: [PATCH 07/11] refactor _from_data_dir --- pymatgen/command_line/chargemol_caller.py | 27 +++++++++++------------ 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/pymatgen/command_line/chargemol_caller.py b/pymatgen/command_line/chargemol_caller.py index 38e9ec96975..b9eba642e0d 100644 --- a/pymatgen/command_line/chargemol_caller.py +++ b/pymatgen/command_line/chargemol_caller.py @@ -191,9 +191,9 @@ def _execute_chargemol(self, **job_control_kwargs): with open(os.path.join(temp_dir, "job_control.txt"), mode="w") as file: file.write(lines) - rs = subprocess.run(CHARGEMOL_EXE, capture_output=True, check=False) - _stdout = rs.stdout.decode("utf-8") - rs.stderr.decode("utf-8") + response = subprocess.run(CHARGEMOL_EXE, capture_output=True, check=False) + _stdout = response.stdout.decode("utf-8") + response.stderr.decode("utf-8") self._from_data_dir(chargemol_output_path=temp_dir) @@ -205,8 +205,7 @@ def _from_data_dir(self, chargemol_output_path=None): Chargemol output files. Default: None (current working directory). """ - if chargemol_output_path is None: - chargemol_output_path = "." + chargemol_output_path = chargemol_output_path or "." charge_path = f"{chargemol_output_path}/DDEC6_even_tempered_net_atomic_charges.xyz" self.ddec_charges = self._get_data_from_xyz(charge_path) @@ -225,21 +224,21 @@ def _from_data_dir(self, chargemol_output_path=None): else: self.ddec_spin_moments = None - rsquared_path = f"{chargemol_output_path}/DDEC_atomic_Rsquared_moments.xyz" - if os.path.isfile(rsquared_path): - self.ddec_rsquared_moments = self._get_data_from_xyz(rsquared_path) + r2_path = f"{chargemol_output_path}/DDEC_atomic_Rsquared_moments.xyz" + if os.path.isfile(r2_path): + self.ddec_rsquared_moments = self._get_data_from_xyz(r2_path) else: self.ddec_rsquared_moments = None - rcubed_path = f"{chargemol_output_path}/DDEC_atomic_Rcubed_moments.xyz" - if os.path.isfile(rcubed_path): - self.ddec_rcubed_moments = self._get_data_from_xyz(rcubed_path) + r3_path = f"{chargemol_output_path}/DDEC_atomic_Rcubed_moments.xyz" + if os.path.isfile(r3_path): + self.ddec_rcubed_moments = self._get_data_from_xyz(r3_path) else: self.ddec_rcubed_moments = None - rfourth_path = f"{chargemol_output_path}/DDEC_atomic_Rfourth_moments.xyz" - if os.path.isfile(rfourth_path): - self.ddec_rfourth_moments = self._get_data_from_xyz(rfourth_path) + r4_path = f"{chargemol_output_path}/DDEC_atomic_Rfourth_moments.xyz" + if os.path.isfile(r4_path): + self.ddec_rfourth_moments = self._get_data_from_xyz(r4_path) else: self.ddec_rfourth_moments = None From 01f0283c1ec0d3302a6336a9ab6d3b8b48cb1687 Mon Sep 17 00:00:00 2001 From: Janosh Riebesell Date: Sun, 21 Apr 2024 15:49:01 -0700 Subject: [PATCH 08/11] test_missing_exe_error --- tests/command_line/test_chargemol_caller.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/tests/command_line/test_chargemol_caller.py b/tests/command_line/test_chargemol_caller.py index 1b3202d5248..89a28729b4a 100644 --- a/tests/command_line/test_chargemol_caller.py +++ b/tests/command_line/test_chargemol_caller.py @@ -1,5 +1,9 @@ from __future__ import annotations +from unittest.mock import patch + +import pytest + from pymatgen.command_line.chargemol_caller import ChargemolAnalysis from pymatgen.core import Element, Structure from pymatgen.util.testing import TEST_FILES_DIR @@ -58,3 +62,13 @@ def test_parse_chargemol2(self): assert chg_mol.summary["ddec"]["spin_moments"] == chg_mol.ddec_spin_moments assert chg_mol.natoms is None assert chg_mol.structure is None + + def test_missing_exe_error(self): + # monkeypatch CHARGEMOL_EXE to raise in ChargemolAnalysis.__init__ + patch.dict("os.environ", {"CHARGEMOL_EXE": "non_existent"}) + + test_dir = f"{TEST_FILES_DIR}/chargemol/spin_unpolarized" + with pytest.raises( + OSError, match="ChargemolAnalysis requires the Chargemol executable to be in PATH. Please download" + ): + ChargemolAnalysis(path=test_dir, run_chargemol=True) From 3f3a43882f584444ad3ea0a084864cb7e48f4f5a Mon Sep 17 00:00:00 2001 From: Yuan Chiang Date: Tue, 23 Apr 2024 16:56:43 -0700 Subject: [PATCH 09/11] remove response --- pymatgen/command_line/chargemol_caller.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/pymatgen/command_line/chargemol_caller.py b/pymatgen/command_line/chargemol_caller.py index b9eba642e0d..3e26723855e 100644 --- a/pymatgen/command_line/chargemol_caller.py +++ b/pymatgen/command_line/chargemol_caller.py @@ -191,9 +191,7 @@ def _execute_chargemol(self, **job_control_kwargs): with open(os.path.join(temp_dir, "job_control.txt"), mode="w") as file: file.write(lines) - response = subprocess.run(CHARGEMOL_EXE, capture_output=True, check=False) - _stdout = response.stdout.decode("utf-8") - response.stderr.decode("utf-8") + subprocess.run(CHARGEMOL_EXE, capture_output=True, check=True) self._from_data_dir(chargemol_output_path=temp_dir) From 43b238501bf03dc7e34189f393eaf5e93b88dd20 Mon Sep 17 00:00:00 2001 From: Yuan Chiang Date: Wed, 15 May 2024 12:45:40 -0700 Subject: [PATCH 10/11] add missing `command_line` --- tests/command_line/test_chargemol_caller.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/command_line/test_chargemol_caller.py b/tests/command_line/test_chargemol_caller.py index 89a28729b4a..29360a4bce9 100644 --- a/tests/command_line/test_chargemol_caller.py +++ b/tests/command_line/test_chargemol_caller.py @@ -11,7 +11,7 @@ class TestChargemolAnalysis: def test_parse_chargemol(self): - test_dir = f"{TEST_FILES_DIR}/chargemol/spin_unpolarized" + test_dir = f"{TEST_FILES_DIR}/command_line/chargemol/spin_unpolarized" chg_mol = ChargemolAnalysis(path=test_dir, run_chargemol=False) assert chg_mol.ddec_charges == [0.8432, -0.8432] assert chg_mol.get_partial_charge(0) == 0.8432 @@ -55,7 +55,7 @@ def test_parse_chargemol(self): assert struct_partial_charge == chg_mol.ddec_charges def test_parse_chargemol2(self): - test_dir = f"{TEST_FILES_DIR}/chargemol/spin_polarized" + test_dir = f"{TEST_FILES_DIR}/command_line/chargemol/spin_polarized" chg_mol = ChargemolAnalysis(path=test_dir, run_chargemol=False) assert chg_mol.ddec_spin_moments == [0.201595, 0.399203, 0.399203] assert chg_mol.summary["ddec"]["bond_order_dict"][0]["bonded_to"][0]["spin_polarization"] == 0.0490 @@ -67,7 +67,7 @@ def test_missing_exe_error(self): # monkeypatch CHARGEMOL_EXE to raise in ChargemolAnalysis.__init__ patch.dict("os.environ", {"CHARGEMOL_EXE": "non_existent"}) - test_dir = f"{TEST_FILES_DIR}/chargemol/spin_unpolarized" + test_dir = f"{TEST_FILES_DIR}/command_line/chargemol/spin_unpolarized" with pytest.raises( OSError, match="ChargemolAnalysis requires the Chargemol executable to be in PATH. Please download" ): From 6973b7ad2b9c4b8c998d0ceba884c9e55fe5d983 Mon Sep 17 00:00:00 2001 From: Yuan Chiang Date: Wed, 15 May 2024 14:15:53 -0700 Subject: [PATCH 11/11] try to use chargemol precompiled binary --- tests/command_line/test_chargemol_caller.py | 32 +++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/tests/command_line/test_chargemol_caller.py b/tests/command_line/test_chargemol_caller.py index 29360a4bce9..a5a21ba3567 100644 --- a/tests/command_line/test_chargemol_caller.py +++ b/tests/command_line/test_chargemol_caller.py @@ -1,6 +1,10 @@ from __future__ import annotations +import os +import subprocess +import zipfile from unittest.mock import patch +from urllib.request import urlretrieve import pytest @@ -8,6 +12,26 @@ from pymatgen.core import Element, Structure from pymatgen.util.testing import TEST_FILES_DIR +ddec_url = "https://sourceforge.net/projects/ddec/files/latest/download" +download_path = TEST_FILES_DIR / "command_line" / "chargemol" / "ddec-latest.gz" +extract_path = TEST_FILES_DIR / "command_line" / "chargemol" + +if not os.path.exists(download_path): + urlretrieve(ddec_url, download_path) + +if not os.path.exists(extract_path / "chargemol_09_26_2017"): + with zipfile.ZipFile(download_path, "r") as zip_ref: + zip_ref.extractall(extract_path) + +exe_path = extract_path / "chargemol_09_26_2017/chargemol_FORTRAN_09_26_2017/compiled_binaries/linux" + +current_path = os.environ.get("PATH", "") +if str(exe_path) not in current_path: + os.environ["PATH"] = str(exe_path) + os.pathsep + current_path + +subprocess.call(["chmod", "+x", str(exe_path / "Chargemol_09_26_2017_linux_serial")]) +subprocess.call(["chmod", "+x", str(exe_path / "Chargemol_09_26_2017_linux_parallel")]) + class TestChargemolAnalysis: def test_parse_chargemol(self): @@ -72,3 +96,11 @@ def test_missing_exe_error(self): OSError, match="ChargemolAnalysis requires the Chargemol executable to be in PATH. Please download" ): ChargemolAnalysis(path=test_dir, run_chargemol=True) + + def test_chargemol_custom_path(self): + chg_mol = ChargemolAnalysis( + path=TEST_FILES_DIR / "command_line" / "bader", + atomic_densities_path=extract_path / "chargemol_09_26_2017" / "atomic_densities", + run_chargemol=True, + ) + print(chg_mol.ddec_charges)