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

CLI: Fix bug in aiida-quantumespresso workflow launch pw-base #1039

Merged
merged 2 commits into from
Sep 11, 2024

Conversation

sphuber
Copy link
Contributor

@sphuber sphuber commented Jul 10, 2024

The command would not specify the CONTROL key for the input parameters resulting in an exception in the upload step. This was not discovered by tests because the workflow is not actually run and the exception is only hit when the calcjob is actually executed.

The command would not specify the `CONTROL` key for the input parameters
resulting in an exception in the upload step. This was not discovered by
tests because the workflow is not actually run and the exception is only
hit when the calcjob is actually executed.
@sphuber sphuber changed the title CLI: Fix bug in aiida-quantumespresso workflow launc pw-base CLI: Fix bug in aiida-quantumespresso workflow launch pw-base Jul 10, 2024
@sphuber
Copy link
Contributor Author

sphuber commented Jul 10, 2024

@mbercx the exception in question is hit here

calculation_type = control_nl['calculation']

It essentially means that the CONTROL.calculation key is required for the PwCalculation, even though it is made required by the BasePwCpInputGenerator class. I wonder if we therefore should not add this check to the validator of the PwCalculation class itself instead of letting it except in the base class when the upload task is executed.

Copy link

@GeigerJ2 GeigerJ2 left a comment

Choose a reason for hiding this comment

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

LGTM!

@sphuber sphuber merged commit ea76d9b into main Sep 11, 2024
13 checks passed
@sphuber sphuber deleted the fix/cli-workflows-pw-base branch September 11, 2024 21:27
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.

2 participants