[WIP] Refactoring to include outputs parsing #80
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mbercx
force-pushed
the
develop
branch
3 times, most recently
from
6d45efa to
35a667f
Compare
…s. Made many functions as public since they could be useful to users and outside of their context.
…onversions should take place. In all other files, ase and pymatgen should not be used.
…ibility regarding the number of output files for each type of computation, e.g. NEB.
Several small changes are made to the `PwOutput` class: 1. Use `pathlib` instead of `os.path` for path definitions. 2. `.from_dir` method: Only look for files from the specified directory using list comprehension. 3. Adapt approach for identifying the `stdout` file to only use the first 5 lines to look for the `Program PWSCF` string. 4. Remove the try-except loop that simply catches a general exception and raises a `ValueError` instead. This would obscure the actual error without an immediate benefit. 5. Define the `d_out_xxx` variables as empty dictionaries before parsing, and redefine them in case the data is parsed successfully. 6. Allow the user to pass a `Path` instead of a string to specify the directory to parse from.
Currently the `PwStdoutParser` nor its parent class `BaseOutputFileParser` parse any of the Quantum ESPRESSO `stdout`. Here we add the `BaseStdoutParser` class that has some very minimal parsing features for parsing the code name and version, as well as the walltime of the calculation. A utility function is also added to convert the Quantum ESPRESSO walltime output into seconds. The constructor of the `BaseOutputFileParser` is also adapted to remove the default to `None`. Since there is currently no way of providing the output content after construction, it should be provided at that time for the class to be useful.
The first schema used for the XML output of Quantum ESPRESSO `pw.x` was still missing. Here it is added, along with a set of tests for the XML parsing of default runs.
|
@gbrunin thanks again for the hard work in restructuring the package. I've added some basic Now I'm adding some basic documentation, also on the design decisions. We should consider these carefully, particularly the UX, since it's hard to change these afterwards. I don't care too much about breaking backwards compatibility if we can converge on a more user-friendly tool. |
|
Also quick shoutout to @elinscott and his package for QE inputs: https://github.com/elinscott/qe_input_prototype He's already indicated he'd be fine with us integrating that work into |
Closed
|
@gbrunin I've also added some basic documentation and design notes. I'm still not quite sure about some of the APIs. I do like the general design now though, I'm just also wondering if:
I've also made a little image to represent the design:
|
…ec to utils. Renamed BaseStdoutParser.parse_stdout_base to parse for compatibility with parent abstract class.
This currently serves no real purpose, as the executable can be easily identified from the class itself. In case we do find a use case later, it can always be added again easily.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I have changed the structure of the code to better reflect the distinction between input and output parsers. Concerning the inputs, all the tests that were already there still pass, it's only a matter of adapting the namespace. For the outputs, I created objects such as
PwOutput, that inherit from an abstractBaseOutput, that for now can be instantiated with afrom_dirmethod (afrom_filesmethod could/should be added as well). A user with a job that ran in a given directory could get the outputs easily using this classmethod. In thesefrom_dirmethods, specific XML and/or standard outputParserswould be used to get the results. EachParserwould parse a single file, and the logic of parsing and extracting the outputs from the different codes would have to be implemented in eachfrom_dirmethod. For instance, aNebOutput.from_dirwould parse the standard output of the global computation and probably the standard outputs and/or XML files for each image. The extracted outputs would be stored as a simple dictionary and these objects would not rely on any external package.Then, in
qe_tools.extractors, ASE and pymatgen objects could be constructed (e.g., ase.Atoms/pymatgen.Structure, band structures,...), allowing each to be optional dependencies.This is only the base logic of the new structure and many things remain to be implemented. The idea now is to see what breaks with this in aiida-quantumespresso and how the parsing could be moved from there to here. Then, more will be added depending on the needs.