search

payu-org / payu

A workflow management tool for numerical models on the NCI computing systems
Apache License 2.0
19 stars 26 forks source link

Added force option to setup and run #256

Closed aidanheerdegen closed 4 years ago

aidanheerdegen commented 4 years ago

Added force option to setup and run. Automatically removes existing work directory. Closes #255.

Also hanged default of reproduce option to False from None.

pep8speaks commented 4 years ago

Hello @aidanheerdegen! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! :beers:

Comment last updated at 2020-05-08 09:26:20 UTC
aidanheerdegen commented 4 years ago

This isn't finished yet. Have to make a test. Also haven't made output overwrite. @marshallward do we want to add that too? I'm broadly in favour, just wanted to check.

marshallward commented 4 years ago

I don't know how output override would work, since if a run crashed then there ought not be an output directory. (If there were then I'd consider it a bug).

But if there's some problem I can't see that you think it will solve, then I reckon it's alright.

aidanheerdegen commented 4 years ago

Not really. The instructions say you can payu run -i n but don't mention you would have to remove the output00n directory to do this. Same with the reproduce flag. You can use it to re-run an experiment, but it won't work if there is an existing output directory.

aidanheerdegen commented 4 years ago

So it is solving a different problem.

marshallward commented 4 years ago

OK, then I could see how it could be useful.

I had never used the payu run -i n stuff much, but there ought to be better support for navigating the history, and wiping and re-running old jobs would certainly be a part of that.

aidanheerdegen commented 4 years ago

Maybe do that in a different PR I guess, rather than overload this one right now?

marshallward commented 4 years ago

Sounds good, smaller is better!

aidanheerdegen commented 4 years ago

Still have to add a test, which I'll do tomorrow as I am off for now. Thanks for the feedback @marshallward

aidanheerdegen commented 4 years ago

Well I went a bit mad and added a bunch of command line argument tests, which did show up a weird errant option that was not a bug but was not correct.

Pretty happy with the resulting tests. Not testing all commands, but easy enough to add them in if you feel so inclined.

aidanheerdegen commented 4 years ago

I reckon I'll just go ahead and merge this @marshallward