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

CalcJobOutputFollower: Fetch retrieved output file if available #269

Merged
merged 1 commit into from
Sep 20, 2022

Conversation

danielhollas
Copy link
Contributor

When the CalcJob is finished, we can fetch the output file from the retrieved folder instead of always going through the remote_folder. Besides performance reasons, this works better when the remote folder is cleared.

Note that I have tested this only in the context of my own AiiDAlab app where I am reusing this code, but I see no reason why it shouldn't work here as well (please test).

Besides performance reasons, this work better
when the remote folder is cleared.
@danielhollas
Copy link
Contributor Author

@csadorf I believe you did this part of the code in case you want to review this.

@csadorf csadorf self-requested a review September 8, 2022 09:26
Copy link
Member

@csadorf csadorf left a comment

Choose a reason for hiding this comment

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

LGTM, but I would appreciate if @unkcpz could also have a quick look at this. Thanks!

@csadorf csadorf requested a review from unkcpz September 8, 2022 10:30
Copy link
Member

@unkcpz unkcpz left a comment

Choose a reason for hiding this comment

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

Thanks, @danielhollas. Only a minor request if we want this widget more generic for other calcjob.

Comment on lines +301 to +302
self.filename = calcjob.attributes["output_filename"]
with calcjob.outputs.retrieved.open(self.filename) as f:
Copy link
Member

Choose a reason for hiding this comment

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

I think this is based on the output_filename in the retrieved list, in this case, the aiida.out (or maybe it is the default file to retrieve, can you double check? @danielhollas ). It is okay for the pw.x calculation. But if we want to make this widget more general for example move into widgets-base in the future, it is better to check that the output_filename is in the retrieve list.

Copy link
Member

Choose a reason for hiding this comment

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

This should be fine for the QE app, we can consider this after we want to make this widget more general by moving it to AWB in the future.

@unkcpz unkcpz merged commit 6d24a37 into aiidalab:master Sep 20, 2022
@unkcpz
Copy link
Member

unkcpz commented Sep 20, 2022

@danielhollas thanks! Cheers.

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

3 participants