From 252efa7839e337c78b8633d32a3961aea949f58a Mon Sep 17 00:00:00 2001 From: Thomas Purcell Date: Tue, 6 Aug 2024 08:53:23 -0700 Subject: [PATCH] Fix d2k function (#3932) * Fix d2k function Need to use the transpose of the inverse matrix for recipmatrix * breaking: snake_case method args kpt_density and recip_cell (better to do early while user base small) * d2k_recip_cell fix kpt_density type anno and doc str * Add test for aims density conversions * Fix mypy errors * Add print line to see what the actual errors look like All tests pass locally * for supercell round coords should fix error in tests * remove extra print line * refactor test_static_si_no_kgrid --------- Co-authored-by: Janosh Riebesell --- src/pymatgen/io/aims/sets/base.py | 64 ++++++++---------- src/pymatgen/util/testing/aims.py | 15 +++- .../static-no-kgrid-si/control.in.gz | Bin 1244 -> 1162 bytes .../static-no-kgrid-si/geometry.in.gz | Bin 216 -> 188 bytes .../static-no-kgrid-si/parameters.json | 4 +- .../aims/test_sets/test_static_generator.py | 6 +- 6 files changed, 49 insertions(+), 40 deletions(-) diff --git a/src/pymatgen/io/aims/sets/base.py b/src/pymatgen/io/aims/sets/base.py index 52ceed9b182..5f266a21a0c 100644 --- a/src/pymatgen/io/aims/sets/base.py +++ b/src/pymatgen/io/aims/sets/base.py @@ -5,7 +5,6 @@ import copy import json import logging -from collections.abc import Iterable from dataclasses import dataclass, field from typing import TYPE_CHECKING, Any from warnings import warn @@ -19,7 +18,7 @@ from pymatgen.io.core import InputFile, InputGenerator, InputSet if TYPE_CHECKING: - from collections.abc import Sequence + from collections.abc import Iterable, Sequence from pymatgen.util.typing import PathLike @@ -234,7 +233,7 @@ def _read_previous( prev_dir (str or Path): The previous directory for the calculation """ prev_structure: Structure | Molecule | None = None - prev_parameters = {} + prev_params = {} prev_results: dict[str, Any] = {} if prev_dir: @@ -243,7 +242,7 @@ def _read_previous( # jobflow_remote) split_prev_dir = str(prev_dir).split(":")[-1] with open(f"{split_prev_dir}/parameters.json") as param_file: - prev_parameters = json.load(param_file, cls=MontyDecoder) + prev_params = json.load(param_file, cls=MontyDecoder) try: aims_output: Sequence[Structure | Molecule] = read_aims_output( @@ -256,7 +255,7 @@ def _read_previous( except (IndexError, AimsParseError): pass - return prev_structure, prev_parameters, prev_results + return prev_structure, prev_params, prev_results @staticmethod def _get_properties( @@ -308,12 +307,9 @@ def _get_input_parameters( Returns: dict: The input object """ - # Get the default configuration - # FHI-aims recommends using their defaults so bare-bones default parameters - parameters: dict[str, Any] = { - "xc": "pbe", - "relativistic": "atomic_zora scalar", - } + # Get the default config + # FHI-aims recommends using their defaults so bare-bones default params + params: dict[str, Any] = {"xc": "pbe", "relativistic": "atomic_zora scalar"} # Override default parameters with previous parameters prev_parameters = {} if prev_parameters is None else copy.deepcopy(prev_parameters) @@ -327,25 +323,25 @@ def _get_input_parameters( kpt_settings["density"] = density parameter_updates = self.get_parameter_updates(structure, prev_parameters) - parameters = recursive_update(parameters, parameter_updates) + params = recursive_update(params, parameter_updates) # Override default parameters with user_params - parameters = recursive_update(parameters, self.user_params) - if ("k_grid" in parameters) and ("density" in kpt_settings): + params = recursive_update(params, self.user_params) + if ("k_grid" in params) and ("density" in kpt_settings): warn( "WARNING: the k_grid is set in user_params and in the kpt_settings," " using the one passed in user_params.", stacklevel=1, ) - elif isinstance(structure, Structure) and ("k_grid" not in parameters): + elif isinstance(structure, Structure) and ("k_grid" not in params): density = kpt_settings.get("density", 5.0) even = kpt_settings.get("even", True) - parameters["k_grid"] = self.d2k(structure, density, even) - elif isinstance(structure, Molecule) and "k_grid" in parameters: + params["k_grid"] = self.d2k(structure, density, even) + elif isinstance(structure, Molecule) and "k_grid" in params: warn("WARNING: removing unnecessary k_grid information", stacklevel=1) - del parameters["k_grid"] + del params["k_grid"] - return parameters + return params def get_parameter_updates( self, @@ -366,7 +362,7 @@ def get_parameter_updates( def d2k( self, structure: Structure, - kptdensity: float | list[float] = 5.0, + kpt_density: float | tuple[float, float, float] = 5.0, even: bool = True, ) -> Iterable[float]: """Convert k-point density to Monkhorst-Pack grid size. @@ -376,15 +372,15 @@ def d2k( Args: structure (Structure): Contains unit cell and information about boundary conditions. - kptdensity (float | list[float]): Required k-point + kpt_density (float | list[float]): Required k-point density. Default value is 5.0 point per Ang^-1. even (bool): Round up to even numbers. Returns: dict: Monkhorst-Pack grid size in all directions """ - recipcell = structure.lattice.inv_matrix - return self.d2k_recipcell(recipcell, structure.lattice.pbc, kptdensity, even) + recip_cell = structure.lattice.inv_matrix.transpose() + return self.d2k_recip_cell(recip_cell, structure.lattice.pbc, kpt_density, even) def k2d(self, structure: Structure, k_grid: np.ndarray[int]): """Generate the kpoint density in each direction from given k_grid. @@ -398,36 +394,36 @@ def k2d(self, structure: Structure, k_grid: np.ndarray[int]): Returns: dict: Density of kpoints in each direction. result.mean() computes average density """ - recipcell = structure.lattice.inv_matrix - densities = k_grid / (2 * np.pi * np.sqrt((recipcell**2).sum(axis=1))) + recip_cell = structure.lattice.inv_matrix.transpose() + densities = k_grid / (2 * np.pi * np.sqrt((recip_cell**2).sum(axis=1))) return np.array(densities) @staticmethod - def d2k_recipcell( - recipcell: np.ndarray, + def d2k_recip_cell( + recip_cell: np.ndarray, pbc: Sequence[bool], - kptdensity: float | Sequence[float] = 5.0, + kpt_density: float | tuple[float, float, float] = 5.0, even: bool = True, ) -> Sequence[int]: """Convert k-point density to Monkhorst-Pack grid size. Args: - recipcell (Cell): The reciprocal cell + recip_cell (Cell): The reciprocal cell pbc (Sequence[bool]): If element of pbc is True then system is periodic in that direction - kptdensity (float or list[floats]): Required k-point - density. Default value is 3.5 point per Ang^-1. + kpt_density (float or list[floats]): Required k-point + density. Default value is 5 points per Ang^-1. even(bool): Round up to even numbers. Returns: dict: Monkhorst-Pack grid size in all directions """ - if not isinstance(kptdensity, Iterable): - kptdensity = 3 * [float(kptdensity)] + if isinstance(kpt_density, float): + kpt_density = (kpt_density, kpt_density, kpt_density) kpts: list[int] = [] for i in range(3): if pbc[i]: - k = 2 * np.pi * np.sqrt((recipcell[i] ** 2).sum()) * float(kptdensity[i]) + k = 2 * np.pi * np.sqrt((recip_cell[i] ** 2).sum()) * float(kpt_density[i]) if even: kpts.append(2 * int(np.ceil(k / 2))) else: diff --git a/src/pymatgen/util/testing/aims.py b/src/pymatgen/util/testing/aims.py index 057df663083..1220974a8b5 100644 --- a/src/pymatgen/util/testing/aims.py +++ b/src/pymatgen/util/testing/aims.py @@ -84,6 +84,7 @@ def comp_system( generator_cls: type, properties: list[str] | None = None, prev_dir: str | None | Path = None, + user_kpt_settings: dict[str, Any] | None = None, ) -> None: """Compare files generated by tests with ones in reference directories. @@ -96,16 +97,24 @@ def comp_system( generator_cls (type): The class of the generator properties (list[str] | None): The list of properties to calculate prev_dir (str | Path | None): The previous directory to pull outputs from + user_kpt_settings (dict[str, Any] | None): settings for k-point density in FHI-aims Raises: AssertionError: If the input files are not the same """ + if user_kpt_settings is None: + user_kpt_settings = {} + k_point_density = user_params.pop("k_point_density", 20) try: - generator = generator_cls(user_params=user_params, k_point_density=k_point_density) + generator = generator_cls( + user_params=user_params, + k_point_density=k_point_density, + user_kpoints_settings=user_kpt_settings, + ) except TypeError: - generator = generator_cls(user_params=user_params) + generator = generator_cls(user_params=user_params, user_kpoints_settings=user_kpt_settings) input_set = generator.get_input_set(structure, prev_dir, properties) input_set.write_input(work_dir / test_name) @@ -117,7 +126,7 @@ def compare_single_files(ref_file: str | Path, test_file: str | Path) -> None: """Compare single files generated by tests with ones in reference directories. Args: - ref_file (str | Path): The reference file to cmpare against + ref_file (str | Path): The reference file to compare against test_file (str | Path): The file to compare against the reference Raises: diff --git a/tests/io/aims/aims_input_generator_ref/static-no-kgrid-si/control.in.gz b/tests/io/aims/aims_input_generator_ref/static-no-kgrid-si/control.in.gz index 9b912b2ffe2d5259c08c4cdf514696c335e18134..f151b8f5b882ff195c5b19737603d0209fbe2daa 100644 GIT binary patch literal 1162 zcmV;51asJh%O9R-fzGWA?hqOh| zUKR^v(W^mABbyB+3M7@*{q;K}ZO32Py|~K3;XKapz2R`kyFYLHML%o!zi(J!q)~D! zUWBpYEeCE@C+g)dr8$_IORl5m58(03*BKX`fm$^P zgI3?Mwhs9AaSD%G*k7|Re1#gm_LM3++9{-Qya>)u<9dz8VgnN?)*Cy423tN#YoJvc zo`laKyt4{c6rB61qNic4daIRWqK8&Uym!$)BA8Jl5ZtJ7C2>}7uzm*KZ$YE+Vb#iV z(NmNk!fwEB(2x^aEBbX4tj4SUOpxs@Q$`v=lxQK=Y ziD{h1@8iY$cn%LuLkqXij(u%~>K)O&Zr)n*)+l;2U_ez1pHW&4mQn<2w6*Hcg*_>k ze>Sw<4{k_uk6rE$cAg-%hL@#lcO5ieoYkV=%;VQ(*3{)I27X^Nz*P7hkVbIyhDyrSm(xkB7C{d$N`mO66!0m}3zPs;9?8vO8T5)-R zsUSoze6}J7_hT(_G$*{1+(R8Vr*`_%Ig95-5#9M2a=GE!YQ&=%`3|c`yBw4~@y=Lr zw2B)fEcUZ~O)eW8mgJ|})=$9MKA7f>;BrZ`UYE*KD0bdExT8uJgA`PygkzWI|YC&qVINg`X z37XH7m;`0{RkSEl3S2DWt7w_dbAYP4j#hD10W7L(kyDn(<(!)ND=Vsc)cgMLye`aM z!mX>x=#m1_4N6;1Q_q$GuF=Jk!t-f`HuipZ9epC)YFgd_ zM5YlPYV*NWa{&)@H?uhWq`P4-Xa4s$70Tq%F$7D!Wdbv&ORJY~k9>exQ6+u(7yY;JvwHG2iWUbup4fnMfWBet!sYn{HVc-PWY_OczuGb}(a|yWT_yBn?-7aC1%exbv|PicqFSqlx-A?a zgZkU1r?2XTXhF&1KAqoZDW!3m4eoB2%&j8r1O6)M|GsX;2bxP}#fvajyyf6ZC(7k- zr5Tx$Nu~$S%e5BOe+Ey|@c;Ii4eoCI4e*a%$yBO}Q>WDrURtO4{$WIqTG-#?&uqmt zeQ8lF+uIR15HEuBQ@>twPq!1$;ah}m(Rr7Eop9iShccDw21Ok*bUhYH*n(Cigq3L?3A%% zq)~EXARM{CJX%Mig9i=m8CKR-g!5e^%M~-ixQHeSDT46vBwkG7IXzUByCqcjZE1yS z9ngK=thVshT(qX+L91%|#HD3qQ6f^kEphi;*b~Bh+hD(+%pm2K`wW`Md?@QWzH<$(5XhrG`Coa~5=u3bZYI$8^;iYB)oA*-CDyQ7AM2)$!Jdm#}Ln zC6~P6t-V0`mA`Ne6N##h4!L;8e@65)B0zoTu%aXeDg?KP1C@ABtm`;#&uK&${^-G| zdW)@*VYyMFytfg8pc~FB8q7}Yc1E+5LQu5xM;7)R{YM42d z<4fmJJTHpDowtz74bxV0eq@pFbY*Xsow6t1SyUYD;uc91duz|hWyQNC{8U|g13umd z)2tFqE@Ab$Ri09@^WLc&FkN&~fTi&E8rH5s4(*%TJ{ez%TJUNR)|G3Lj&DS9RLrMS z{87Z$l#6KsWs+P`CQ+K^3;ZtfYszf4K(<+&?kRGj&F4vsgtGjqwkT2rE*9}s?KGX| zM6=m-?JSBBxOvPv>aluc*L!uJ;qbd0mKo3b$sj??Rsr%yg|`*kI8xISkbJ z6L%)${K6!6ZD!XMEuIPCX6zM#xuA_N*;Q2+R3V^Gpe*~2wdHoOlPn=QiIhZFRb5cs zKG)Mx#;VF!8jT_3z3=xXqDMDjw5s~h;*a~QpER&kjq9MUpEl%LdYBBXRgh?mLJXh= zGm1phDQy`{&vqfExr-%*hf@n}?EBp``Ut(%*xf0Br4kL-=H8LHpa+cRC=NgAZW+Xx z|NTQe89q9MWYJoNG@vvEC+*g_jP-8$PVwX*Slah%u>zW zlc=|@_qqF{Ix8CS+uu9n(JEoWjlnBLMo~n6xdiZHJ5L}7gz2>erWc3osPHeQl%W8% G4FCY{Gku!? diff --git a/tests/io/aims/aims_input_generator_ref/static-no-kgrid-si/geometry.in.gz b/tests/io/aims/aims_input_generator_ref/static-no-kgrid-si/geometry.in.gz index c2e720a036642f8b6363fb1f551ee7ed40bb9911..413cb3eb22ac036ee72ce497d1484cbb709f79f0 100644 GIT binary patch literal 188 zcmV;t07L&DiwFqC?yzP617~G#ZDn+Fc`j*g0F{wJ3WP8WMfaYf`+<;ZW1Vx%0Ysz= zK@p_P`DYeG8PgDBmY>hd&(AvLyv%8Qr#bJ3a|rl1>4E563(?c1)~pOg5IT;M@U^c( z4d4S$AZazenex8LdyU#_`7vxY=nSb7V^3-h>`LYHKv~lM6V_jc@G$7c&CvA>QX=mnQ-Q z7fBGVc8n^BWQq3?OEMOKNBpC3!#RM6vN-*#{h*d&et0jGA3m5Lu)Nn(yUG_Mj*>J> Szx|$?V>|&0*)$P=0ssK=4qhk# diff --git a/tests/io/aims/aims_input_generator_ref/static-no-kgrid-si/parameters.json b/tests/io/aims/aims_input_generator_ref/static-no-kgrid-si/parameters.json index 28242f36fa7..a6c81a5f1e6 100644 --- a/tests/io/aims/aims_input_generator_ref/static-no-kgrid-si/parameters.json +++ b/tests/io/aims/aims_input_generator_ref/static-no-kgrid-si/parameters.json @@ -4,7 +4,7 @@ "species_dir": "/home/tpurcell/git/atomate2/tests/aims/species_dir/light", "k_grid": [ 12, - 12, - 12 + 6, + 4 ] } diff --git a/tests/io/aims/test_sets/test_static_generator.py b/tests/io/aims/test_sets/test_static_generator.py index 39415758b1e..a38c002897d 100644 --- a/tests/io/aims/test_sets/test_static_generator.py +++ b/tests/io/aims/test_sets/test_static_generator.py @@ -19,7 +19,11 @@ def test_static_si(tmp_path): def test_static_si_no_kgrid(tmp_path): parameters = {"species_dir": "light"} - comp_system(Si, parameters, "static-no-kgrid-si", tmp_path, ref_path, StaticSetGenerator) + Si_supercell = Si.make_supercell([1, 2, 3], in_place=False) + for site in Si_supercell: + # round site.coords to ignore floating point errors + site.coords = [round(x, 15) for x in site.coords] + comp_system(Si_supercell, parameters, "static-no-kgrid-si", tmp_path, ref_path, StaticSetGenerator) def test_static_o2(tmp_path):