search

payu-org / payu

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

Refactor experiment.set_counter() and some minor bug fixes #366

Closed jo-basevi closed 9 months ago

jo-basevi commented 9 months ago

Closes #285, closes #307, closes #306

pep8speaks commented 9 months ago

Hello @jo-basevi! 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 2023-10-03 23:26:51 UTC
coveralls commented 9 months ago

Coverage Status

coverage: 42.053% (+0.04%) from 42.016% when pulling b5c0c303ca2f1909ab7b2ce541bbcc6b5d24bff6 on jo-basevi:285-fix-crash-with-restart into edf8d8c538040447dcd3bdd944cba5bab86937c1 on payu-org:master.

jo-basevi commented 9 months ago

A previous CI action failed at the lint stage because of payu/experiment.py:564:20: E1111: Assigning result of a function call, where the function has no return (assignment-from-no-return) where the code was in the payu run command:

for prof in self.profilers:
      if prof.runscript:
          model_prog = model_prog.append(prof.runscript)

I think it found a legitimate error so I fixed it in the previous commit (though could be wrong). But I am bit confused as to how that error isn't picked up when running pylint --extension-pkg-whitelist=netCDF4 --ignored-modules=backports -E payu locally or in other CI builds in other PRs..

aidanheerdegen commented 9 months ago

I think it found a legitimate error so I fixed it in the previous commit (though could be wrong). But I am bit confused as to how that error isn't picked up when running pylint --extension-pkg-whitelist=netCDF4 --ignored-modules=backports -E payu locally or in other CI builds in other PRs..

Yeah that is weird. It is a not often used part of the code, but you're right that a limiter should have picked that up before. Is computing supposed to be deterministic?

jo-basevi commented 9 months ago

Yeah that is weird. It is a not often used part of the code, but you're right that a limiter should have picked that up before. Is computing supposed to be deterministic?

Well it is now failing in another PR, so it'll be good to fix here..