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

XspectraCalculation+XspectraParser: Add Features for Restarts and Other Updates #837

Merged
merged 36 commits into from Sep 27, 2022

Conversation

PNOGillespie
Copy link
Contributor

Adds features necessary for restarting XSpectra calculations:

  • Reserves the save file name (xanes.sav) in the codeinfo object.
  • The code now automatically includes xanes.sav in the local_copy_list if the parent calculation is an XspectraCalculation (taken from the parent folder), thus simplifying the setup for calculation restarts.

Other additions included in the PR:

  • A new error code for the XspectraParser (code 315) to detect if the code has cleanly exited after reaching a set maximum time limit (including an accompanying test).
  • A slight increase to the output_parameters collected: adds the polarisation vectors chosen for the calculation and whether the calculation was set to re-plot (i.e. xonly_plot = .true./.false. in the inputs).
  • Support for including an optional gamma_file (SinglefileData format) required to run calculations using an external file for applying spectral broadening to the final spectrum (using gamma_mode = "file").

PNOGillespie and others added 25 commits May 9, 2022 10:58
Added CalcJob implementations of the xspectra.x executable and upf2plotcore.sh shell script from the Quantum ESPRESSO library
Added Parser classes for the xspectra.x executable and upf2plotcore.sh shell script from the Quantum ESPRESSO library
Added entry points for both CalcJob and Parser classes required for XSpectra post-processing calculations
Change 'spectra_data' output to 'spectra'

Co-authored-by: Sebastiaan Huber <mail@sphuber.net>
Change the caplitalization for XSpectraCalculation to XspectraCalculation

Co-authored-by: Sebastiaan Huber <mail@sphuber.net>
Correct the capitalization of XSpectraParser to XspectraParser

Co-authored-by: Sebastiaan Huber <mail@sphuber.net>
Correct the capitalization of XSpectraCalculation to XspectraCalculation

Co-authored-by: Sebastiaan Huber <mail@sphuber.net>
Correct the capitalization of XSpectraParser to XspectraParser

Co-authored-by: Sebastiaan Huber <mail@sphuber.net>
…ved redundant file loop, and added new error code
Fixed the terms used in the XSpectra-specific error
codes in order to make them easier to read.
Changed filename assigned to `_Plotcore_FILENAME`
from `plotcore.out` to `stdout` in order to work
properly with the `aiida-shell` plugin.
Made corrections to the parsing of various output
parameters (e.g. energy levels and associated units)
in both spin-polarised and unpolarised cases. An
additional parameter `lsda` is also added to report
explicitly the case of spin polarisation.
Adds tests of the `XspectraParser`, covering spin-polarised
and non-spin-polarised cases, interrupted calculations, and
XSpectra-specific error codes 313 and 314.
Adds the following features to the `XspectraCalculation` and
`XspectraParser` classes:
- A save file ('xanes.sav') is now reserved by default and is printed
at the end of each calculation run or after a designated time limit.
The file can then be used for re-starting halted calculations or
re-plotting spectra from previously-finished calculations.
- The `XspectraCalculation` class now copies the save file from a
parent calculation if the calculation type is itself an
XspectraCalculation, thus re-starts and re-plot runs are handled
automatically.
- An error code has been added (315) for calculations which safely exit
after reaching their time limit, thus enabiling a restart.
- Additional information is now included in the `output_parameters`
node (e.g. the polarisation vectors used and whether the calculation was
set to "xonly_plot").
Updates `XspectraParser` tests to work with changes made in a previous
commit (commit 875fa58) and adds a new
test for error code 315 (job exceeded time limit)
Copy link
Contributor

@sphuber sphuber left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @PNOGillespie . Just some minor comments but overall the changes look good.

src/aiida_quantumespresso/calculations/xspectra.py Outdated Show resolved Hide resolved
src/aiida_quantumespresso/calculations/xspectra.py Outdated Show resolved Hide resolved
src/aiida_quantumespresso/calculations/xspectra.py Outdated Show resolved Hide resolved
src/aiida_quantumespresso/calculations/xspectra.py Outdated Show resolved Hide resolved
src/aiida_quantumespresso/parsers/xspectra.py Outdated Show resolved Hide resolved
tests/parsers/test_xspectra.py Outdated Show resolved Hide resolved
PNOGillespie and others added 4 commits September 12, 2022 10:09
Addresses aiidateam#837

Co-authored-by: Sebastiaan Huber <mail@sphuber.net>
Addresses PR aiidateam#837

Co-authored-by: Sebastiaan Huber <mail@sphuber.net>
Addresses PR aiidateam#837

Changes error code from 315 to 400 to match conventions

Co-authored-by: Sebastiaan Huber <mail@sphuber.net>
Addresses PR aiidateam#837

Required for commit aiidateam@746770e

Co-authored-by: Sebastiaan Huber <mail@sphuber.net>
PNOGillespie and others added 2 commits September 12, 2022 10:17
Addresses PR aiidateam#837

Required with commit aiidateam@746770e

Co-authored-by: Sebastiaan Huber <mail@sphuber.net>
Addresses PR aiidateam#837

Cleans-up the code for detecting if the parent calculation was an `XspectraCalculation`

Co-authored-by: Sebastiaan Huber <mail@sphuber.net>
@PNOGillespie
Copy link
Contributor Author

Thank you @sphuber for the review, I will put out a new commit after my meetings today to shorten the out-of-walltime test file.

Regarding the discussion here: #837 (comment) - The original formulation included FolderData as a means to provide a parent calculation for the calculation run, however the use of this wasn't tested before and I'm not familiar with the procedure for retrieving the files required in order to use the FolderData node properly for this purpose. I have opted to simply remove the option from the inputs for now and then add it back in via a later commit once I understand how this process works.

PNOGillespie and others added 3 commits September 12, 2022 14:42
Addresses comment for PR aiidateam#837: aiidateam#837 (review)

Safely trims down the file length of the stdout file for `failed_timeout` test of `XspectraParser`
Copy link
Contributor

@sphuber sphuber left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @PNOGillespie . Just one minor mistake in the test docstring and one outstanding comment. If you can address those, this can be merged.

Edit: never mind, I see you addressed that comment regarding the FolderData in a separate post. Just the docstring then and I will merge this.

tests/parsers/test_xspectra.py Outdated Show resolved Hide resolved
Correct text of docstring for `test_xspectra_failed_timeout`

Co-authored-by: Sebastiaan Huber <mail@sphuber.net>
@PNOGillespie
Copy link
Contributor Author

Thank you for the reply @sphuber (and for spotting that mistake). I've addressed the requested change, so I'll leave the rest to you.

@sphuber sphuber merged commit 17d4bd6 into aiidateam:main Sep 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants