Skip to content

Commit 4a012d5

Browse files
committed
AiiDAConverter: add full_dos conversion
As a demonstrator of the motivation of having a `full_dos` output, we add a conversion mapping from `full_dos` to an AiiDA `XyData`. It should be clear that this mapping can be improved: we're missing a true `DosData` data type, which could also store the Fermi energy. However, here we show the advantage of separating the "extraction" to base outputs and "conversion" into library ones: we can have a single converter class that takes care of the conversion of outputs of the various Quantum ESPRESSO codes. While working on this, some other considerations regarding the design came to mind. These are added to corresponding design section.
1 parent 80a523e commit 4a012d5

File tree

2 files changed

+46
-1
lines changed

2 files changed

+46
-1
lines changed

docs/design/outputs.md

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -159,13 +159,27 @@ class PymatgenConverter(BaseConverter):
159159
},
160160
),
161161
}
162-
163162
```
164163

165164
However, if this is not the case, the output cannot be directly constructed with this approach.
166165
An example here is AiiDA's `StructureData`.
167166
This points to poor design of this class' constructor, but we can still support the class by allowing the first element in the now `(<output_converter>, <glom_spec>)` tuple to be a function.
168167

168+
!!! note
169+
170+
This approach requires careful syncing the `_output_spec_mapping` of the output classes to the `conversion_mapping` of the converter classes, and hence the code logic for obtaining is not fully localized.
171+
To make things worse, in some cases it also requires understanding the raw outputs (but this can be prevented with clear schemas for the base outputs).
172+
We're not fully converged on the design here, but some considerations below:
173+
174+
1. If we want the code for converting to a certain library to be isolated, we will always have to accept some delocalization.
175+
We could consider directly extracting the data required from the raw outputs, but then a developer still has to go check the corresponding output class for the keys it uses to store the raw output, as well as the raw output itself.
176+
Moreover: it could lead to a lot of code duplication; right now the base outputs are already in the default units.
177+
178+
2. One other issue could be name conflicts: in case there are multiple outputs from different output classes that have the same name but different content, you cannot define conversion (or lack thereof) for both of them.
179+
However, it seems clear that we should try to have consistent and distinct names for each output.
180+
181+
At this stage, we think clearly structured and defined "base outputs" are a better approach than direct extraction.
182+
169183
## User interface
170184

171185
A `get_output` method is implemented on the `PwOutput`, which is the main user-facing interface for all these features.

src/qe_tools/converters/aiida.py

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
import numpy as np
44

5+
from glom import T
56
from qe_tools.converters.base import BaseConverter
67

78
try:
@@ -23,6 +24,21 @@ def _convert_structure_data(cell, symbols, positions):
2324
return structure
2425

2526

27+
def _convert_dos(energy: list, dos: list | dict):
28+
xy_data = orm.XyData()
29+
xy_data.set_x(np.array(energy), "energy", "eV")
30+
if isinstance(dos, dict):
31+
xy_data.set_y(
32+
[np.array(dos["dos_down"]), np.array(dos["dos_up"])],
33+
["dos_spin_down", "dos_spin_down"],
34+
["states/eV", "states/eV"],
35+
)
36+
else:
37+
xy_data.set_y(np.array(dos), "dos", "states/eV")
38+
39+
return xy_data
40+
41+
2642
class AiiDAConverter(BaseConverter):
2743
conversion_mapping = {
2844
"structure": (
@@ -33,4 +49,19 @@ class AiiDAConverter(BaseConverter):
3349
"positions": ("positions", lambda positions: np.array(positions)),
3450
},
3551
),
52+
"full_dos": (
53+
_convert_dos,
54+
{
55+
"energy": "energy",
56+
"dos": (
57+
T,
58+
lambda full_dos: full_dos["dos"]
59+
if "dos" in full_dos
60+
else {
61+
"dos_down": full_dos["dos_down"],
62+
"dos_up": full_dos["dos_up"],
63+
},
64+
),
65+
},
66+
),
3667
}

0 commit comments

Comments
 (0)