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

Add arithmetic workflows and restructure calculations #4124

Merged
merged 2 commits into from
May 28, 2020

Conversation

mbercx
Copy link
Member

@mbercx mbercx commented May 27, 2020

Overhaul of the arithmetic processes for demonstration purposes and the documentation revamp.

Refactors the calculations package to remove the plugin directory. Also changes the ArithmeticAddCalculation to add defaults for the resources, and change the required code to bash.

Introduces new workflows for demonstration purposes, consistent with the revamped documentation:

  • the multiply_add module with the MultiplyAddWorkChain class and required multiply calculation function.
  • the add_multiply module with the add_and_multiply work function an AddMultiplyWorkChain class.

@mbercx mbercx requested a review from sphuber May 27, 2020 16:26
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 @mbercx , few suggestions and comments

aiida/calculations/plugins/arithmetic/add.py Outdated Show resolved Hide resolved
aiida/calculations/arithmetic/add.py Outdated Show resolved Hide resolved
aiida/calculations/arithmetic/add.py Outdated Show resolved Hide resolved
aiida/calculations/arithmetic/add.py Outdated Show resolved Hide resolved
aiida/calculations/arithmetic/add.py Outdated Show resolved Hide resolved
aiida/workflows/arithmetic/multiply_add.py Outdated Show resolved Hide resolved
aiida/workflows/arithmetic/multiply_add.py Outdated Show resolved Hide resolved
aiida/workflows/arithmetic/multiply_add.py Outdated Show resolved Hide resolved
aiida/workflows/arithmetic/multiply_add.py Outdated Show resolved Hide resolved
aiida/workflows/arithmetic/multiply_add.py Outdated Show resolved Hide resolved
@mbercx mbercx requested a review from sphuber May 28, 2020 08:29
@sphuber sphuber force-pushed the arithmetic branch 5 times, most recently from 80d1e7b to 9c84cec Compare May 28, 2020 13:05
Refactor the calculations package to remove the plugin directory. The
calculation job plugins that ship with `aiida-core` were being stored in
`aiida.calculations.plugins` but the `plugins` submodule is unnecessary
so it is removed. The old import is kept for now and will emit a
deprecation warning.

The `ArithemeticAddCalculation` is updated to simplify its usage as well
as to make it easier to understand the implementation, which is
important as it is used consistenly as an example in tutorials and the
documentation. The following changes are applied:

 * `metadata.options.resources` has a default
 * Unnecessary utility methods are removed to reduce amount of code
 * `calc_info.cmdline_params` are replaced with `calc_info.stdin_name`
 * Code will now work directly with bash instead of custom `add.sh` script

Finally, two workflows implementations are added for testing and
demonstration purposes. The `add_multiply` work function and the
`MultiplyAddWorkChain`. These will be used extensively in the how-to
guides of the documentation and the tutorials.
@sphuber sphuber force-pushed the arithmetic branch 2 times, most recently from ea73747 to a85bd4c Compare May 28, 2020 14:22
Add also tests for the `ArithmeticAddCalculation` which is not new but
was up till now untested. Now that this will start to be used heavily in
tutorials and documentation for teaching purposes, we should make sure
that it is well tested. Here we add an integration test and a unit test
for the `prepare_for_submission` method.
@sphuber
Copy link
Contributor

sphuber commented May 28, 2020

@mbercx Can you rebase this PR so that the docs are build?

@csadorf this PR is not for docs-revamp but goes straight into develop. I will take care of this and once it is done, it will unblock Marnik's PR for the docs-revamp

@sphuber sphuber merged commit 10a5cdb into aiidateam:develop May 28, 2020
@csadorf
Copy link
Contributor

csadorf commented May 28, 2020

@mbercx Can you rebase this PR so that the docs are build?

@csadorf this PR is not for docs-revamp but goes straight into develop. I will take care of this and once it is done, it will unblock Marnik's PR for the docs-revamp

Sorry, I didn't pay enough attention when I made that comment.

@mbercx mbercx deleted the arithmetic branch May 28, 2020 22:03
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.

3 participants