search

sphuber / aiida-shell

AiiDA plugin that makes running shell commands easy.
MIT License
14 stars 7 forks source link

Consider changing the filenames of the `stdout` and `stderr` redirects and the `status` file #1

Open sphuber opened 2 years ago

sphuber commented 2 years ago

It is quite possible that a user might want to run a command that uses these as output files, which is currently forbidden due to the name clash. It might be best to change those names to something more unusual, maybe like __stdout for example.

giovannipizzi commented 9 months ago

Of course using a different name is a good default option, I think a single underscore would be enough, mirroring e.g. _aiidasubmit.sh (I would vote for _stdout.txt, i.e. with the extension).

Wouldn't it be better (if not there yet), though, to also allow in the Metadata options to also let the user optionally choose another one? Similar to the submit script. This will give maximum flexibility to users for cases we didn't think about, as it happened in #58

sphuber commented 9 months ago

Wouldn't it be better (if not there yet), though, to also allow in the Metadata options to also let the user optionally choose another one? Similar to the submit script.

In principle it is there, it is the output_filename option that is defined by the CalcJob base class. I do already use that to set the CodeInfo.stdout_name in the implementation, but I doubt it is fully plummed through and not sure it works.

The problem is that the output file names are directly used as the output link labels. Here full stops are replaced by underscores, since they are not valid characters in link labels. So stdout.txt would be wrapped in a SinglefileData node and be attached with the stdout_txt label. This would pose a problem for files starting with underscores, because a link cannot start with an underscore. That means we would have to strip it, but then what happens when there are both _stdout and stdout as output files?

This will give maximum flexibility to users for cases we didn't think about, as it happened in https://github.com/sphuber/aiida-shell/issues/58

This is not really a problem due to the choice of the default filenames. This would have happened regardless of the choice, also _stdout.txt. The problem here is when using RemoteData nodes, its contents overwrite the files written by the new job. This problem is addressed in this PR https://github.com/sphuber/aiida-shell/pull/70

So currently, I don't really see a reason yet to change the default filenames. If I do, the default output label in the most simple case won't be stdout anymore, but stdout_txt, or a lot of one-off logic would have to be added just to handle the link labels for these default output files, which is also not desirable I think.