Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update docs and add extra error handling #63

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,8 @@ print(results['parameters'].get_dict())

The geometry of the structure is fully defined through the `structure` input, which is provided by a `StructureData`
node. Any other properties, e.g., the charge and what basis set to use, can be specified through the `structure`
dictionary in the `parameters` input:
dictionary in the `parameters` input. A specific SCF solver can also be specified using the `solver` keyword. For
example:

```python
from ase.build import molecule
Expand All @@ -142,7 +143,10 @@ from aiida.orm import Dict, StructureData, load_code
builder = load_code('pyscf').get_builder()
builder.structure = StructureData(ase=molecule('H2O'))
builder.parameters = Dict({
'mean_field': {'method': 'RHF'},
'mean_field': {
'method': 'RHF',
'solver': 'CDIIS',
},
'structure': {
'basis ': 'sto-3g',
'charge': 0,
Expand Down
8 changes: 8 additions & 0 deletions src/aiida_pyscf/calculations/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,7 @@ def validate_parameters(cls, value: Dict | None, _) -> str | None: # noqa: PLR0

if (scf_solver := mean_field.get('solver')) is not None:
valid_scf_solvers = ('DIIS', 'CDIIS', 'EDIIS', 'ADIIS')
forbidden_scf_solvers = ('SOSCF', 'NEWTON')
options = ' '.join(valid_scf_solvers)

if scf_solver is None:
Expand All @@ -183,6 +184,13 @@ def validate_parameters(cls, value: Dict | None, _) -> str | None: # noqa: PLR0
scf_solver = 'CDIIS'
return '`DIIS` is an alias for CDIIS in PySCF. Using `CDIIS` explicitly instead.'

# When PySCF adds support for pickling SOSCF objects, we can remove this stanza
if scf_solver.upper() in forbidden_scf_solvers:
return (
f'The solver `{scf_solver.upper()}` specified in `mean_field.solver` parameters is not yet '
f'supported. Choose from: {options}'
)

if scf_solver.upper() not in valid_scf_solvers:
return (
f'Invalid solver `{scf_solver}` specified in `mean_field.solver` parameters. Choose from: {options}'
Expand Down
13 changes: 13 additions & 0 deletions tests/calculations/test_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,19 @@ def test_invalid_parameters_mean_field_solver_diis(generate_calc_job, generate_i
with pytest.raises(ValueError, match=r'`DIIS` is an alias for CDIIS in PySCF. Using `CDIIS` explicitly instead.'):
generate_calc_job(PyscfCalculation, inputs=inputs)

@pytest.mark.parametrize(
'solver, expected', (
({'solver': 'newton'}, 'The solver `NEWTON` specified in `mean_field.solver` parameters is not yet supported.'),
({'solver': 'sOsCf'}, 'The solver `SOSCF` specified in `mean_field.solver` parameters is not yet supported.'),
)
)
def test_invalid_parameters_mean_field_solver_second_order(generate_calc_job, generate_inputs_pyscf, solver, expected):
"""Test logic to catch second order solver input for ``parameters.mean_field.solver``."""
parameters = {'mean_field': solver}
inputs = generate_inputs_pyscf(parameters=parameters)
with pytest.raises(ValueError, match=expected):
generate_calc_job(PyscfCalculation, inputs=inputs)


def test_invalid_parameters_mean_field_chkfile(generate_calc_job, generate_inputs_pyscf):
"""Test validation of ``parameters.mean_field.chkfile``, is not allowed as set automatically by plugin."""
Expand Down