-
Notifications
You must be signed in to change notification settings - Fork 82
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
Adding ENVIRON namelists #703
base: main
Are you sure you want to change the base?
Changes from 7 commits
a991ec6
d2691b8
a134570
d753459
726b355
64161bc
12be851
9fc3a1a
b5d2859
29e0574
894b95a
e38ddf1
8f0bb86
1f6dd1b
db7d722
f657f45
9ef55c5
4168fee
5009842
3d13abb
90aae90
acb2d0a
c8016b8
fe50cfa
5764e4f
ed7e70c
94d1e85
2e5bc1b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,146 @@ | ||
# -*- coding: utf-8 -*- | ||
|
||
from aiida import orm | ||
from aiida.engine import run, submit | ||
import numpy as np | ||
from ase import Atoms | ||
from ase import units | ||
""" | ||
Calculation that recreates the environ calculation of | ||
CO on Pt(111) with a unit charge | ||
|
||
More details about the solvation models and potential corrections | ||
can be found here: | ||
|
||
O. Andreussi, I. Dabo and N. Marzari, J. Chem. Phys. 136,064102 (2012). | ||
I. Dabo et al. Phys.Rev. B 77, 115139 (2008) | ||
|
||
This system is taken directly from Example04 in the | ||
examples folder from Environ | ||
""" | ||
|
||
|
||
def calculator(): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why did you make this and the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yup good point, has been corrected. |
||
""" | ||
This is the SCF calculator, which is identical | ||
and unchanged from what one would do in the case | ||
of a standard SCF calculation - nothing environ | ||
related. Remember to add in the tot_charge here | ||
""" | ||
param_dict = { | ||
'CONTROL': { | ||
'calculation': 'scf', | ||
'tprnfor': True, | ||
}, | ||
'SYSTEM': { | ||
'ecutwfc': 35, | ||
'ecutrho': 280, | ||
'occupations': 'smearing', | ||
'smearing': 'cold', | ||
'degauss': 0.03, | ||
'tot_charge': 1, | ||
}, | ||
'ELECTRONS': { | ||
'conv_thr': 5e-9, | ||
'electron_maxstep': 200, | ||
'mixing_beta': 0.2, | ||
'mixing_ndim': 15, | ||
'diagonalization': 'david', | ||
}, | ||
} | ||
|
||
return param_dict | ||
|
||
|
||
def environ_calculator(): | ||
""" | ||
Environ parameters | ||
""" | ||
param_dict = { | ||
'ENVIRON': { | ||
'verbose': 0, | ||
'environ_thr': 1.0, | ||
'environ_type': 'input', | ||
'env_static_permittivity': 1, | ||
'env_surface_tension': 0.0, | ||
'env_pressure': 0.0, | ||
'env_electrostatic': True, | ||
}, | ||
'BOUNDARY': { | ||
'solvent_mode': 'full', | ||
}, | ||
'ELECTROSTATIC': { | ||
'pbc_correction': 'parabolic', | ||
'pbc_dim': 2, | ||
'pbc_axis': 3, | ||
'tol': 5e-13, | ||
} | ||
} | ||
return param_dict | ||
|
||
|
||
def runner(code, structure): | ||
|
||
## Base calculation for testing | ||
PwBaseWorkChain = WorkflowFactory('quantumespresso.pw.base') | ||
builder = PwBaseWorkChain.get_builder() | ||
builder.pw.structure = structure | ||
|
||
builder.metadata.label = 'Single point environ' | ||
builder.metadata.description = 'Testing calculation with environ for Pt(111) + CO' | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This hardcodes the structure even though the structure is mutable through the argument. Not a huge deal since it is an example, but still a bit weird. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yup, also corrected. |
||
|
||
KpointsData = DataFactory('array.kpoints') | ||
kpoints = KpointsData() | ||
kpoints.set_kpoints_mesh([1, 1, 1]) # Not physical just a test | ||
builder.kpoints = kpoints | ||
|
||
family = load_group('SSSP/1.1/PBE/efficiency') | ||
builder.pw.pseudos = family.get_pseudos(structure=structure) | ||
|
||
calculation = calculator() | ||
environ_calculation = environ_calculator() | ||
## these are the main cube files that could potentially be parsed | ||
## if the verbosity is set to 2 or higher | ||
# environ_calculation['additional_retrieve_list'] = ['epsilon.cube', \ | ||
# 'vreference.cube', 'velectrostatic.cube', 'vsoftcavity.cube', \ | ||
# 'electrons.cube', 'charges.cube', 'smeared_ions.cube'] | ||
|
||
builder.pw.parameters = orm.Dict(dict=calculation) | ||
builder.pw.settings = orm.Dict(dict=environ_calculation) | ||
builder.pw.metadata.options.resources = {'num_machines': 1} | ||
builder.pw.metadata.options.max_wallclock_seconds = 5 * 60 | ||
builder.pw.code = code | ||
|
||
calculation = submit(builder) | ||
|
||
|
||
if __name__ == '__main__': | ||
code = load_code('pw_6-7@juwels') | ||
|
||
StructureData = DataFactory('structure') | ||
## these are the original coordinates for the Pt-CO system | ||
positions = [ | ||
[5.335084148, 4.646723426, 12.901029877], | ||
[5.335009643, 4.619623254, 15.079854269], | ||
[8.061327071, 0.098057998, 8.992142901], | ||
[2.608989366, 0.098058283, 8.992140585], | ||
[0.000036609, 4.720846294, 8.968756935], | ||
[5.335159557, 4.721612729, 9.380196435], | ||
[0.000041121, 7.802951963, 4.604626508], | ||
[5.335161233, 7.697749113, 4.753489408], | ||
[2.697860636, 3.152173889, 4.688412329], | ||
[7.972463687, 3.152174491, 4.688415209], | ||
] | ||
|
||
## setting up the system with ASE | ||
## notice the units that are being used | ||
atoms = Atoms('COPt8') | ||
atoms.set_positions(np.array(positions) * units.Bohr) | ||
atoms.set_pbc([True, True, True]) | ||
a = 10.6881 * units.Bohr | ||
b = 0.866025 * a * units.Bohr | ||
c = 3.95422 * a * units.Bohr | ||
atoms.set_cell([a, b, c]) | ||
structure = StructureData(ase=atoms) | ||
|
||
runner(code=code, structure=structure) |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -250,3 +250,26 @@ def test_pw_parallelization_duplicate_cmdline_flag(fixture_sandbox, generate_cal | |
with pytest.raises(InputValidationError) as exc: | ||
generate_calc_job(fixture_sandbox, entry_point_name, inputs) | ||
assert 'Conflicting' in str(exc.value) | ||
|
||
|
||
def test_environ_namelists(fixture_sandbox, generate_calc_job, generate_inputs_pw): | ||
"""Test that Environ namelists are created.""" | ||
entry_point_name = 'quantumespresso.pw' | ||
|
||
inputs = generate_inputs_pw() | ||
inputs['settings'] = orm.Dict( | ||
dict={ | ||
'ENVIRON': { | ||
'electrolyte_linearized': True, | ||
'environ_type': 'input', | ||
}, | ||
'BOUNDARY': { | ||
'solvent_mode': 'electronic', | ||
'electrolyte_mode': 'electronic', | ||
}, | ||
'ELECTROSTATIC': { | ||
'pbc_correction': 'parabolic', | ||
} | ||
}, | ||
) | ||
generate_calc_job(fixture_sandbox, entry_point_name, inputs) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This test is not actually yet testing that the namelists are (properly) generates. The
The There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hi @sphuber - thanks a lot for the comments! I have a few questions about how these tests work. I have added in Regarding the parser, I modeled the Environ test after the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If you just add the
No, it doesn't run the calculation. I set up the framework for these parser tests that it simply mocks an already completed calculation with outputs taking from the test directory. Each test has its own folder that corresponds to the name of the test. So all you have to do is add a folder for your test with the output files of a run you did manually with the environ module enabled. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks a lot! So I think I understand now how the tests for the calculations work (and have updated the latest commit with the right files). The same thing seems to work for the parser, but I am not sure I am testing this properly. All inputs to Environ would be passed through the settings, so I am not sure if this information is included in the yml that is generated. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The tests now look fine 👍 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Great, thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add a log here as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks Giovanni! I have added it to the log in the most recent commit - thanks!
https://github.com/sudarshanv01/aiida-quantumespresso/blob/894b95a9e44fda4875b0a4f3fcd580bd40242e29/aiida_quantumespresso/parsers/parse_raw/pw.py#L753
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just for my understanding: is the string "Gaussian-smeared nuclei" specific to ENVIRON calculations? Because if that is the case, the warning message in the except case, doesn't make a lot of sense, does it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yup, I guess it is specific to ENVIRON calculations, right now it says
Not an Environ calculation with an open boundary condition
, but this might show up for all calculations, which might be a bit annoying? Is there a better way to have a log message here without it popping up every time - and only for ENVIRON calculations?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, just add a conditional to the
elif 'Gaussian-smeared nuclei' in line
to only run when it is an ENVIRON calculation. How to determine whether we actually are parsing an ENVIRON question depends. Probably you could parse the stdout for some flag, but this is really fragile. If the XML doesn't contain some switch that we can rely on (which would be the best and most robust solution) you can use the input parameters. If I understand correctly, if you want to run an ENVIRON calculation, you have to add a particular key to theparameters
input node. You can check for that at the beginning of the parser. Something likeand then the parsing line would become
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, absolutely - just put this change in. In the current form I would need to pass an extra argument of settings to the
parse_raw
function. I could also just sendis_environ
as an argument, but maybe it is more complete to pass thesettings
dictionary, just likeparameters
?