search

aiidateam / aiida-wannier90

AiiDA plugin for the Wannier90 code
https://aiida-wannier90.readthedocs.io
Other
9 stars 15 forks source link

Remove opening transport in 'prepare_for_submission' #83

Closed greschd closed 4 years ago

greschd commented 4 years ago

Fixes #37 and fixes #76

To avoid opening a transport to the remote_input_folder, we use globbing to specify all possible input files that can be given. Because we can no longer determine if there are overlaps between local and remote input, the two inputs are now mutually exclusive. Also, the check for the required input files is performed only when a local input is passed. Because passing an input does not make sense in the postproc_setup mode, this is no longer allowed.

The implementation separates the specification of input files (which are optional / required, should be copied / symlinked) - which is shared between local and remote input - from the generation of the input lists.

TODO: The remote_input_folder version was tested with local transport, but should probably also be tested with ssh transport. It's a somewhat unfortunate consequence of the split between input testing and parser testing that it can't check if the syntax of the input lists is correct (only that it didn't change).

greschd commented 4 years ago

I also took the opportunity to slightly refactor the prepare_for_submission - validation of the input and output names was moved to a separate function.

greschd commented 4 years ago

@giovannipizzi I'm still tweaking this a little bit.

giovannipizzi commented 4 years ago

Quick question - do you want to keep both self.inputs.local_input_folder and self.inputs.remote_input_folder in the end, or you are going to change that part in a different PR? (see #76)

greschd commented 4 years ago

I would keep them both, to keep a path open for implementing mixed input in a backwards-compatible way should it become necessary.

greschd commented 4 years ago

I'm still tweaking this a little bit.

Good to go.

giovannipizzi commented 4 years ago

Note that there might be a small conflict with some changes done in some PRs just merged (on the example for GaAs) - up to you if you want to rebase or merge and fix the confict (if any)

greschd commented 4 years ago

Note that there might be a small conflict with some changes done in some PRs just merged (on the example for GaAs) - up to you if you want to rebase or merge and fix the confict (if any)

Yeah I had seen that on your PR - we just both fixed the same typo. Nothing complicated, so I'll just merge.

greschd commented 4 years ago

Ok, I've fixed it according to your comments. Note also that it's now using the explicit file name (instead of globbing) for the required input files -- in that case, it's actually desired that it fails when the file is not there.