Implement `CalcJob` process class #2389

sphuber · 2019-01-15T16:16:26Z

Fixes #2377, fixed #2280, fixes #2219 and fixed #2381

This commit can be summarized in three steps:

Reimplementation of a job calculation as Process called CalcJob
Changing job calculation to be purely informational
Remove the old job calculation mechanics and business logic

The old way of creating a job calculation, was to subclass the
JobCalculation class and override the _use_methods class method to
define the input nodes and the _prepare_for_submission to setup the
input files for the calculation. The problem was that these methods were
implemented on the Node class, thus mixing the responsabilities of
running and introspecting the results of a completed calculation.

Here we define the CalcJob class, a subclass of Process. This class
replaces the old JobCalculation and allows a user to defined the
inputs and outputs through the ProcessSpec, just as they would do for
a WorkChain. Except, instead of defining an outline, one should
implement the prepare_for_submission, which fulfills the exact same
function as before, only it is now a public method of the CalcJob
process class.

Finally, the role of the job calculation state, stored as an attribute
with the key state on the CalcJobNode has changed significantly.
The original job calculations had a calculation state that controlled
the logic during its lifetime. This was already superceded a long time
ago by the process wrapper that now fully governs the progression of the
calculation. Despite the calculation state no longer being authoritative
during the calculation's lifetime, it was still present. Here we finally
fully remove and only leave a stripped down version. The remaining state
is stored as an attribute and is a sub state while the CalcJob process
is in an active state and serves as a more granual state that can be
queried for. This is useful, because the process status, which also
keeps similar information is human readable and doesn't allow for easy
querying.

coveralls · 2019-01-15T16:43:21Z

Coverage increased (+0.8%) to 69.503% when pulling 70f5ff6 on sphuber:fix_2377_implement_calc_job_process into de14357 on aiidateam:provenance_redesign.

This commit can be summarized in three steps: * Reimplementation of a job calculation as `Process` called `CalcJob` * Changing job calculation to be purely informational * Remove the old job calculation mechanics and business logic The old way of creating a job calculation, was to subclass the `JobCalculation` class and override the `_use_methods` class method to define the input nodes and the `_prepare_for_submission` to setup the input files for the calculation. The problem was that these methods were implemented on the `Node` class, thus mixing the responsabilities of running and introspecting the results of a completed calculation. Here we define the `CalcJob` class, a subclass of `Process`. This class replaces the old `JobCalculation` and allows a user to defined the inputs and outputs through the `ProcessSpec`, just as they would do for a `WorkChain`. Except, instead of defining an `outline`, one should implement the `prepare_for_submission`, which fulfills the exact same function as before, only it is now a public method of the `CalcJob` process class. Finally, the role of the job calculation state, stored as an attribute with the key `state` on the `CalcJobNode` has changed significantly. The original job calculations had a calculation state that controlled the logic during its lifetime. This was already superceded a long time ago by the process wrapper that now fully governs the progression of the calculation. Despite the calculation state no longer being authoritative during the calculation's lifetime, it was still present. Here we finally fully remove and only leave a stripped down version. The remaining state is stored as an attribute and is a sub state while the `CalcJob` process is in an active state and serves as a more granual state that can be queried for. This is useful, because the process status, which also keeps similar information is human readable and doesn't allow for easy querying.

giovannipizzi

Just a couple minor things

giovannipizzi · 2019-01-17T12:20:10Z

aiida/backends/djsite/db/migrations/0023_calc_job_option_attribute_keys.py

+            SET key =  regexp_replace(attribute.key, '^environment_variables', 'custom_environment_variables')
+            FROM db_dbnode AS node
+            WHERE
+                attribute.key like 'environment_variables%' AND


A few notes here

you should escape the underscore (should be \_)

maybe better to do something like (attribute.key='environment_variables' OR attribute.key like 'environment\_variables.%')

Similar for the others where applicable (where it's a single variable and not a dict/list, you don't need to do like instead, just =)

giovannipizzi · 2019-01-17T12:34:18Z

aiida/orm/node/process/calculation/calcjob.py

-            raise ModificationNotAllowed('invalid link: CalcJobNode has to have state in {}, but is {}'.format(
-                valid_states, state))
-
-    def validate_outgoing(self, target, link_type, link_label):


Don't we want to still do some validation depending on the (process_)state? Or it's somewhere else?

The `replace_*link_from` methods where only implemented because they were needed by the old job calculation created. Since that system is now removed, this functionality is no longer needed. Likewise, the methods to remove links, either from the cache or the database were not used nor tested and so they are removed.

When a `Node` instance is sealed, it should be impossible to add links to or from it. To enforce this, the `validate_incoming` and `validate_outgoing` methods are overridden in the `Sealable` mixin that will raise `ModificationNotAllowed` if the node is sealed.

In the pattern operands for the `LIKE` operator, the underscore is a special character that matches any character. If a literal underscore should be matched, it should be escaped by a backslash.

giovannipizzi · 2019-01-17T14:26:57Z

Wonderful!

sphuber force-pushed the fix_2377_implement_calc_job_process branch from 0bf4e16 to fa3652c Compare January 15, 2019 16:17

sphuber force-pushed the fix_2377_implement_calc_job_process branch from fa3652c to 1d4d305 Compare January 15, 2019 16:49

sphuber requested a review from giovannipizzi January 15, 2019 17:29

sphuber force-pushed the fix_2377_implement_calc_job_process branch 2 times, most recently from 5c35c74 to 62638b4 Compare January 16, 2019 08:45

sphuber force-pushed the fix_2377_implement_calc_job_process branch from 62638b4 to 72083b6 Compare January 17, 2019 12:35

giovannipizzi requested changes Jan 17, 2019

View reviewed changes

sphuber added 3 commits January 17, 2019 14:03

Properly escape underscores in LIKE operands

70f5ff6

In the pattern operands for the `LIKE` operator, the underscore is a special character that matches any character. If a literal underscore should be matched, it should be escaped by a backslash.

sphuber force-pushed the fix_2377_implement_calc_job_process branch from d0b6c67 to 70f5ff6 Compare January 17, 2019 14:25

giovannipizzi approved these changes Jan 17, 2019

View reviewed changes

sphuber merged commit 2c961cb into aiidateam:provenance_redesign Jan 17, 2019

sphuber deleted the fix_2377_implement_calc_job_process branch January 17, 2019 14:48

sphuber mentioned this pull request Feb 26, 2019

Process interface: namespace the default process inputs store_provenance, label and description #2180

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement `CalcJob` process class #2389

Implement `CalcJob` process class #2389

sphuber commented Jan 15, 2019 •

edited

Loading

coveralls commented Jan 15, 2019 •

edited

Loading

giovannipizzi left a comment

giovannipizzi Jan 17, 2019

giovannipizzi Jan 17, 2019

giovannipizzi commented Jan 17, 2019

Implement CalcJob process class #2389

Implement CalcJob process class #2389

Conversation

sphuber commented Jan 15, 2019 • edited Loading

coveralls commented Jan 15, 2019 • edited Loading

giovannipizzi left a comment

Choose a reason for hiding this comment

giovannipizzi Jan 17, 2019

Choose a reason for hiding this comment

giovannipizzi Jan 17, 2019

Choose a reason for hiding this comment

giovannipizzi commented Jan 17, 2019

Implement `CalcJob` process class #2389

Implement `CalcJob` process class #2389

sphuber commented Jan 15, 2019 •

edited

Loading

coveralls commented Jan 15, 2019 •

edited

Loading