Skip to content

Commit

Permalink
Don't auto-set charge/spin in Q-Chem calculator (#908)
Browse files Browse the repository at this point in the history
## Overview

Previously, the Q-Chem calculator (and by extension, the Q-Chem recipes)
tried to automatically set the spin multiplicity if the user did not
provide it. This was done using the function
`quacc.utils.atoms.get_charge_and_spin` (can also be called as
`quacc.utils.get_charge_and_spin` as a result of this PR).

In this PR, I am proposing that we move this logic out of the Q-Chem
calculator. In other words, the following change would be required if
using quacc in a high-throughput context:

```python
from ase.build import molecule
from quacc.recipes.qchem.core import relax_job

atoms = molecule("CH3")
output = relax_job(atoms)
```

would instead become

```python
from ase.build import molecule
from quacc.recipes.qchem.core import relax_job
from quacc.utils import check_charge_and_spin

atoms = molecule("CH3")
charge, spin_multiplicity = check_charge_and_spin(atoms) 
output = relax_job(atoms, charge, spin_multiplicity)
```

## Motivation

As was noted in prior discussions, it is clear that the approach
proposed here is more verbose and slightly more laborious than the
original implementation. There is no denying that. However, I believe
that there are strong pros for this PR.

1. In general, my philosophy is that charge and spin are so crucial that
they are parameters that I want the user to really think about.
Automation is great, but this is one area where I think it is important
the user stops to think about what they're doing rather than blindly
assuming quacc will take care of it for them. I also prefer to take the
"road of least surprise."

2. For transition metal complexes, the prior approach becomes a bit
messy since it will auto-set the spin multiplicity to the lowest
possible value and "just run" but of course, many (in fact, most)
transition metal complexes are not actually low spin. This is in line
with the above point where I'd prefer the user to be explicit about what
they want to model.
 
3. The `check_charge_and_spin` function is extremely useful! And
thankfully, it is simple to use. As noted in the above codeblock,
removing `check_charge_and_spin` from inside the Q-Chem calculator does
not remove the ability to use the function. One can still use it in
their script as they see fit, even in a high-throughput context. It does
require an additional line, but I think this is okay.

---------

Co-authored-by: deepsource-autofix[bot] <62050782+deepsource-autofix[bot]@users.noreply.github.com>
  • Loading branch information
Andrew-S-Rosen and deepsource-autofix[bot] authored Sep 11, 2023
1 parent 047b2e6 commit ecc24a3
Show file tree
Hide file tree
Showing 21 changed files with 332 additions and 239 deletions.
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),

### Changed

- Streamlined handling of spin multiplicity kwargs in molecular DFT calculators.
- Charge and spin multiplicity are now required arguments in molecular DFT calculators.
- Slab recipes now use `make_slabs_from_bulk` instead of `make_max_slabs_from_bulk`
- Use the `logging` module when warnings do not need to be immediately addressed.
- Functions are no longer used as kwargs in recipes to help with (de)serialization in certain workflow engines.
Expand Down
23 changes: 4 additions & 19 deletions src/quacc/calculators/qchem.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
from pymatgen.io.qchem.sets import ForceSet

from quacc.custodian import qchem as custodian_qchem
from quacc.utils.atoms import get_charge_and_spin

logger = logging.getLogger(__name__)

Expand All @@ -30,10 +29,9 @@ class QChem(FileIOCalculator):
cores
Number of cores to use for the Q-Chem calculation.
charge
The total charge of the molecular system. Effectively defaults to zero.
The total charge of the molecular system.
spin_multiplicity
The spin multiplicity of the molecular system. Effectively defaults to
the lowest spin state given the molecular structure and charge.
The spin multiplicity of the molecular system.
qchem_input_params
Dictionary of Q-Chem input parameters to be passed to
pymatgen.io.qchem.sets.ForceSet.
Expand All @@ -52,8 +50,8 @@ class QChem(FileIOCalculator):
def __init__(
self,
atoms: Atoms,
charge: None | int = None,
spin_multiplicity: None | int = None,
charge: int = 0,
spin_multiplicity: int = 1,
method: str | None = None,
cores: int = 1,
qchem_input_params: dict | None = None,
Expand All @@ -74,9 +72,6 @@ def __init__(
if "overwrite_inputs" not in self.qchem_input_params:
self.qchem_input_params["overwrite_inputs"] = {}

if self.charge is None and self.spin_multiplicity is not None:
raise ValueError("If setting spin_multiplicity, must also specify charge.")

if self.qchem_input_params.get("smd_solvent") and self.qchem_input_params.get(
"pcm_dielectric"
):
Expand All @@ -90,16 +85,6 @@ def __init__(
):
self.qchem_input_params["overwrite_inputs"]["rem"]["method"] = method

charge, spin_multiplicity = get_charge_and_spin(
atoms, self.charge, self.spin_multiplicity
)
if charge != self.charge or spin_multiplicity != self.spin_multiplicity:
logger.info(
f"{self.charge, self.spin_multiplicity} for charge, spin multiplicity changed to {charge, spin_multiplicity}",
)
self.charge = charge
self.spin_multiplicity = spin_multiplicity

# We will save the parameters that have been passed to the Q-Chem
# calculator via FileIOCalculator's self.default_parameters
self.default_parameters = {
Expand Down
20 changes: 10 additions & 10 deletions src/quacc/recipes/gaussian/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@
@job
def static_job(
atoms: Atoms | dict,
charge: int = 0,
multiplicity: int = 1,
charge: int,
spin_multiplicity: int,
xc: str = "wb97x-d",
basis: str = "def2-tzvp",
calc_swaps: dict | None = None,
Expand All @@ -41,7 +41,7 @@ def static_job(
the value
charge
Charge of the system.
multiplicity
spin_multiplicity
Multiplicity of the system.
xc
Exchange-correlation functional
Expand All @@ -62,7 +62,7 @@ def static_job(
"xc": xc,
"basis": basis,
"charge": charge,
"mult": multiplicity,
"mult": spin_multiplicity,
"sp": "",
"scf": ["maxcycle=250", "xqc"],
"integral": "ultrafine",
Expand Down Expand Up @@ -90,7 +90,7 @@ def static_job(
"xc": xc,
"basis": basis,
"charge": charge,
"mult": multiplicity,
"mult": spin_multiplicity,
"sp": "",
"scf": ["maxcycle=250", "xqc"],
"integral": "ultrafine",
Expand All @@ -114,8 +114,8 @@ def static_job(
@job
def relax_job(
atoms: Atoms,
charge: int = 0,
multiplicity: int = 1,
charge: int,
spin_multiplicity: int,
xc: str = "wb97x-d",
basis: str = "def2-tzvp",
freq: bool = False,
Expand All @@ -132,7 +132,7 @@ def relax_job(
the value
charge
Charge of the system.
multiplicity
spin_multiplicity
Multiplicity of the system.
xc
Exchange-correlation functional
Expand All @@ -155,7 +155,7 @@ def relax_job(
"xc": xc,
"basis": basis,
"charge": charge,
"mult": multiplicity,
"mult": spin_multiplicity,
"opt": "",
"pop": "CM5",
"scf": ["maxcycle=250", "xqc"],
Expand Down Expand Up @@ -183,7 +183,7 @@ def relax_job(
"xc": xc,
"basis": basis,
"charge": charge,
"mult": multiplicity,
"mult": spin_multiplicity,
"opt": "",
"pop": "CM5",
"scf": ["maxcycle=250", "xqc"],
Expand Down
2 changes: 1 addition & 1 deletion src/quacc/recipes/lj/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ def freq_job(
Returns
-------
FreqSchema
Dictionary of results, specified in `quacc.schemas.ase.FreqSchema`
Dictionary of results, specified in [quacc.schemas.ase.run_ase_vib][]
"""
atoms = fetch_atoms(atoms)
calc_swaps = calc_swaps or {}
Expand Down
16 changes: 8 additions & 8 deletions src/quacc/recipes/orca/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@
@job
def static_job(
atoms: Atoms | dict,
charge: int = 0,
multiplicity: int = 1,
charge: int,
spin_multiplicity: int,
xc: str = "wb97x-d3bj",
basis: str = "def2-tzvp",
input_swaps: dict | None = None,
Expand All @@ -44,7 +44,7 @@ def static_job(
the value
charge
Charge of the system.
multiplicity
spin_multiplicity
Multiplicity of the system.
xc
Exchange-correlation functional
Expand Down Expand Up @@ -120,7 +120,7 @@ def static_job(
atoms.calc = ORCA(
profile=OrcaProfile([SETTINGS.ORCA_CMD]),
charge=charge,
mult=multiplicity,
mult=spin_multiplicity,
orcasimpleinput=orcasimpleinput,
orcablocks=orcablocks,
)
Expand All @@ -136,8 +136,8 @@ def static_job(
@job
def relax_job(
atoms: Atoms | dict,
charge: int = 0,
multiplicity: int = 1,
charge: int,
spin_multiplicity: int,
xc: str = "wb97x-d3bj",
basis: str = "def2-tzvp",
run_freq: bool = False,
Expand All @@ -154,7 +154,7 @@ def relax_job(
Atoms object
charge
Charge of the system.
multiplicity
spin_multiplicity
Multiplicity of the system.
xc
Exchange-correlation functional
Expand Down Expand Up @@ -233,7 +233,7 @@ def relax_job(
atoms.calc = ORCA(
profile=OrcaProfile([SETTINGS.ORCA_CMD]),
charge=charge,
mult=multiplicity,
mult=spin_multiplicity,
orcasimpleinput=orcasimpleinput,
orcablocks=orcablocks,
)
Expand Down
16 changes: 8 additions & 8 deletions src/quacc/recipes/psi4/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@
@requires(psi4, "Psi4 not installed. Try conda install -c psi4 psi4")
def static_job(
atoms: Atoms | dict,
charge: int = 0,
multiplicity: int = 1,
charge: int,
spin_multiplicity: int,
method: str = "wb97x-v",
basis: str = "def2-tzvp",
calc_swaps: dict | None = None,
Expand All @@ -44,7 +44,7 @@ def static_job(
the value
charge
Charge of the system.
multiplicity
spin_multiplicity
Multiplicity of the system.
method
The level of theory to use.
Expand All @@ -64,8 +64,8 @@ def static_job(
"method": method,
"basis": basis,
"charge": charge,
"multiplicity": multiplicity,
"reference": "uks" if multiplicity > 1 else "rks",
"multiplicity": spin_multiplicity,
"reference": "uks" if spin_multiplicity > 1 else "rks",
}
```
copy_files
Expand All @@ -85,8 +85,8 @@ def static_job(
"method": method,
"basis": basis,
"charge": charge,
"multiplicity": multiplicity,
"reference": "uks" if multiplicity > 1 else "rks",
"multiplicity": spin_multiplicity,
"reference": "uks" if spin_multiplicity > 1 else "rks",
}
flags = merge_dicts(defaults, calc_swaps)

Expand All @@ -96,6 +96,6 @@ def static_job(
return summarize_run(
final_atoms,
input_atoms=atoms,
charge_and_multiplicity=(charge, multiplicity),
charge_and_multiplicity=(charge, spin_multiplicity),
additional_fields={"name": "Psi4 Static"},
)
29 changes: 9 additions & 20 deletions src/quacc/recipes/qchem/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
from quacc.calculators.qchem import QChem
from quacc.schemas import fetch_atoms
from quacc.schemas.ase import summarize_opt_run, summarize_run
from quacc.utils.atoms import get_charge_and_spin
from quacc.utils.calc import run_ase_opt, run_calc
from quacc.utils.dicts import merge_dicts, remove_dict_empties

Expand All @@ -30,8 +29,8 @@
@job
def static_job(
atoms: Atoms | dict,
charge: int | None = None,
spin_multiplicity: int | None = None,
charge: int,
spin_multiplicity: int,
method: str = "wb97mv",
basis: str = "def2-tzvpd",
scf_algorithm: str = "diis",
Expand All @@ -49,11 +48,9 @@ def static_job(
Atoms object or a dictionary with the key "atoms" and an Atoms object as
the value
charge
Charge of the system. If None, this is determined from
`quacc.utils.atoms.get_charge_and_spin`
Charge of the system.
spin_multiplicity
Multiplicity of the system. If None, this is determined from
`quacc.utils.atoms.get_charge_and_spin`
Multiplicity of the system.
method
DFT exchange-correlation functional or other electronic structure
method. Defaults to wB97M-V.
Expand Down Expand Up @@ -85,9 +82,6 @@ def static_job(
Dictionary of results from [quacc.schemas.ase.summarize_run][]
"""
atoms = fetch_atoms(atoms)
charge, spin_multiplicity = get_charge_and_spin(
atoms, charge=charge, multiplicity=spin_multiplicity
)

qchem_defaults = {
"method": method,
Expand Down Expand Up @@ -119,8 +113,8 @@ def static_job(
@job
def relax_job(
atoms: Atoms | dict,
charge: int | None = None,
spin_multiplicity: int | None = None,
charge: int,
spin_multiplicity: int,
method: str = "wb97mv",
basis: str = "def2-svpd",
scf_algorithm: str = "diis",
Expand All @@ -139,11 +133,9 @@ def relax_job(
Atoms object or a dictionary with the key "atoms" and an Atoms object as
the value
charge
Charge of the system. If None, this is determined from
`quacc.utils.atoms.get_charge_and_spin`
Charge of the system.
spin_multiplicity
Multiplicity of the system. If None, this is determined from
`quacc.utils.atoms.get_charge_and_spin`
Multiplicity of the system.
method
DFT exchange-correlation functional or other electronic structure
method. Defaults to wB97M-V.
Expand All @@ -169,7 +161,7 @@ def relax_job(
default values set therein as well as set additional Q-Chem parameters.
See QChemDictSet documentation for more details.
opt_swaps
Dictionary of custom kwargs for `run_ase_opt`.
Dictionary of custom kwargs for [quacc.utils.calc.run_ase_opt[]
???+ Note
Expand All @@ -186,9 +178,6 @@ def relax_job(

# TODO: exposing TRICs?
atoms = fetch_atoms(atoms)
charge, spin_multiplicity = get_charge_and_spin(
atoms, charge=charge, multiplicity=spin_multiplicity
)

qchem_defaults = {
"method": method,
Expand Down
Loading

0 comments on commit ecc24a3

Please sign in to comment.