aiida-core icon indicating copy to clipboard operation
aiida-core copied to clipboard

`CalcJob`: Allow to define order of copying of input files

Open sphuber opened this issue 1 year ago • 2 comments
trafficstars

Fixes #6012

There are three different sources of files that are copied to the working directory on the remote computer where a calculation job is executed:

  • Sandbox: files written to a temporary sandbox folder on the local file system written by the CalcJob plugin, first in the prepare_for_submission method, followed by the presubmit of the base class.
  • Local: files written to the temporary sandbox folder by the engine based on the local_copy_list defined by the plugin in the prepare_for_submission method.
  • Remote: files written directly to the remote working directory by the engine base on the remote_copy_list defined by the plugin in the prepare_for_submission method.

Historically, these operations were performed in the order of local, sandbox and remote. For certain use cases, however, this was deemed non-ideal, for example because files from the remote would override files written by the plugin itself in the sandbox. This enum can be assigned to the CalcInfo instance returned by the calculation job plugin prepare_for_submission implementation to change the order.

Here, a new attribute file_copy_order is added to the CalcInfo datastructure. It takes an instance of the FileCopyOrder enum. The value of the enum determines the order in which the type of files as listed above are copied to the working directory. The default is kept to the historical order of LOCAL_SANDBOX_REMOTE, but one more option is added REMOTE_LOCAL_SANDBOX for now.

To simplify the implementation, the code to copy these three types of files are factored out of the upload_calculation function to the functions _copy_remote_files, _copy_remote_files and _copy_sandbox_files functions, respectively.

sphuber avatar Feb 10 '24 23:02 sphuber

This is a proof-of-principle implementation. It is fully backwards-compatible. I still need to add tests and documentation, but already wanted to put this up for review on the concept.

For context: I successfully use this feature to solve this issue in aiida-shell: https://github.com/sphuber/aiida-shell/issues/58

sphuber avatar Feb 10 '24 23:02 sphuber

@unkcpz @mbercx would it be possible to give this PR priority in the review? It is necessary to fix an important problem in aiida-shell that are blocking some users. These changes are fully backwards compatible so it should be fine to release with an upcoming v2.6

sphuber avatar Feb 13 '24 21:02 sphuber

@giovannipizzi @mbercx I have implemented the alternative solution as suggested by you. Please have a look

sphuber avatar Mar 27 '24 07:03 sphuber

Thanks! Approved from my side, not clicking the approve button just because I think we should also add a paragraph to the docs, and 1 test if not too complex (something like putting the same filename with different content and trying two orders and then check what file overwrote which one, if we have tests that can check that)

Thanks. Of course, I will definitely add a test and update the docs, just wanted to first hash out the design, but if this is accepted, I will add those now. I have already tested it with my use-case for aiida-shell where it works exactly as required.

sphuber avatar Mar 27 '24 08:03 sphuber

@mbercx you want to have a final look? Or can I go ahead and merge this?

sphuber avatar Mar 27 '24 13:03 sphuber

Thanks @sphuber! I didn't field test this yet, but changes look good. Feel free to merge and I'll dogfood it later :)

mbercx avatar Mar 27 '24 14:03 mbercx

@unkcpz got another timeout on the Docker build: https://github.com/aiidateam/aiida-core/actions/runs/8455387921/job/23162740045?pr=6285

sphuber avatar Mar 27 '24 17:03 sphuber