-
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
Support calculating the XPS spectra of the atoms specific by indices #958
Conversation
Hi @mhdzbert could you test this PR? Hi @PNOGillespie , you can also try if you are interested in. Feedback is welcomed. Thanks! |
Hi @superstar54. I think this is a good, simple addition which brings some more advanced control to XPS (and other analysis types which rely on a having a marked atom in the structure). Having done a simple test, I can see one necessary change already - plus some other suggestions for the WorkChain. |
atoms_list = orm.List(self.ctx.atoms_list) | ||
inputs = { | ||
'atoms_list' : atoms_list, | ||
'marker' : self.inputs.abs_atom_marker, | ||
'metadata' : { | ||
'call_link_label' : 'get_marked_structures' | ||
} | ||
} | ||
} # populate this further once the schema for WorkChain options is figured out | ||
if 'structure_preparation_settings' in self.inputs: | ||
optional_cell_prep = self.inputs.structure_preparation_settings | ||
for key, node in optional_cell_prep.items(): | ||
inputs[key] = node | ||
if 'spglib_settings' in self.inputs: | ||
spglib_settings = self.inputs.spglib_settings | ||
inputs['spglib_settings'] = spglib_settings | ||
else: | ||
spglib_settings = None | ||
|
||
if 'relax' in self.inputs: | ||
relaxed_structure = self.ctx.relaxed_structure | ||
result = get_xspectra_structures(relaxed_structure, **inputs) | ||
else: | ||
result = get_xspectra_structures(self.inputs.structure, **inputs) | ||
|
||
supercell = result.pop('supercell') | ||
out_params = result.pop('output_parameters') | ||
if out_params.get_dict().get('structure_is_standardized', None): | ||
standardized = result.pop('standardized_structure') | ||
self.out('standardized_structure', standardized) | ||
|
||
# structures_to_process = {Key : Value for Key, Value in result.items()} | ||
for site in ['output_parameters', 'supercell', 'standardized_structure']: | ||
result.pop(site, None) | ||
result = get_marked_structures(input_structure, **inputs) | ||
self.ctx.supercell = input_structure | ||
self.ctx.equivalent_sites_data = result.pop('output_parameters').get_dict() |
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.
Since get_marked_structures()
doesn't return a supercell_structure
or symm_analysis_dict
, these can't be sent to output. This causes a problem, because these two are required outputs - thus the calculation using an atoms_list
input returns error code 11 (missing required output(s)):
Property Value
----------- -------------------------------------------------------------
type XpsWorkChain
state Finished [11] The process did not register a required output.
pk 200
uuid edb735da-d8f8-436f-b3bf-94519255f5f6
label XpsWorkChain (default)
description
ctime 2023-06-27 10:06:51.852633+00:00
mtime 2023-06-27 10:18:55.460074+00:00
Inputs PK Type
------------------------ ---- -------------
ch_scf
clean_workdir 190 Bool
max_iterations 189 Int
kpoints_force_parity 188 Bool
kpoints_distance 187 Float
pw
parallelization 186 Dict
parameters 185 Dict
structure 184 StructureData
code 1 InstalledCode
pseudos
O 12 UpfData
C 32 UpfData
Li 30 UpfData
core_hole_pseudos
O 110 UpfData
C 106 UpfData
Li 107 UpfData
gipaw_pseudos
O 109 UpfData
C 105 UpfData
Li 108 UpfData
abs_atom_marker 191 Str
atoms_list 195 List
calc_binding_energy 192 Bool
clean_workdir 194 Bool
core_hole_treatments 196 Dict
correction_energies 193 Dict
spglib_settings 197 Dict
structure 184 StructureData
voight_gamma 198 Float
voight_sigma 199 Float
Outputs PK Type
------------------------ ---- ------
chemical_shifts
Li_cls 250 Dict
O_cls 251 Dict
C_cls 252 Dict
final_spectra_cls
Li_cls_spectra 253 XyData
O_cls_spectra 254 XyData
C_cls_spectra 255 XyData
output_parameters_ch_scf
site_0 238 Dict
site_4 242 Dict
site_6 246 Dict
Called PK Type
--------------------- ---- ----------------------
get_marked_structures 202 get_marked_structures
site_0_xps 208 PwBaseWorkChain
site_4_xps 210 PwBaseWorkChain
site_6_xps 212 PwBaseWorkChain
compile_final_spectra 249 get_spectra_by_element
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.
Either changing the outputs to not be required, or integrating get_marked_structures()
into get_xspectra_structures()
(which does return this information) should fix the issue.
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 for pointing out this. I changed them to be not required.
One other point: I see that for
For one thing, These points aren't specific for this PR, so I wouldn't ask to address this now, but should be addressed at some point. |
I changed it to
Could explain more about what this means? There is an input parameter called |
def get_builder_from_protocol( | ||
cls, code, structure, pseudos, core_hole_treatments=None, protocol=None, | ||
overrides=None, elements_list=None, options=None, | ||
overrides=None, elements_list=None, atoms_list=None, options=None, | ||
structure_preparation_settings=None, **kwargs | ||
): |
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.
The other point is that both of these should be provided via specific inputs and then mapped to the proper AiiDA types as needed. That way, these optional inputs don't need to be included at all if they're not needed by the user.
Could explain more about what this means? There is an input parameter called calc_binding_energy in the spec.inputs
You have defined these in the process spec, yes, however in get_builder_from_protocol()
, there are no inputs fields in the function itself and instead we must provide both the correction energies and the switch to calculate the binding energies via kwargs
instead - which isn't as easy to use or provide documentation for.
overrides=None, elements_list=None, atoms_list=None, options=None, | ||
structure_preparation_settings=None, **kwargs | ||
): |
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.
Adding at least the correction_energies
in the form of a standard Python dict
object would probably be enough - since they are required for a calculation where calc_binding_energy = True
, the function could just set calc_binding_energy = orm.Bool(True)
if correction_energies
is defined at all. If they aren't defined, then the function should logically set calc_binding_energy = orm.Bool(False)
def get_builder_from_protocol(
...
structure_preparation_settings=None, correction_energies=None, **kwargs
):
if correction_energies:
builder.correction_energies = orm.Dict(correction_energies)
builder.calc_binding_energies = orm.Bool(True)
else:
builder.calc_binding_energies = orm.Bool(False)
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! I changed the code as you suggested.
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.
All good. I ran a quick live-fire test on my end with no issues. The latest change makes this a lot more intuitive to use IMO.
This PR implements the XPS workchain to support calculating the XPS spectra of the atoms specific by indices.
Before, the XPS workchain only supports calculating the XPS spectra for selected elements by analyzing the symmetry and finding the non-equivalent site. This is not suitable for large systems with low symmetry, e.g. supported nanoparticles, in which all atoms are non-equivalent. And user usually only interested in the spectra of some special atoms.
This PR adds a new input
atoms_list
, e,g,