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

Add CABLE model driver #314

Closed aidanheerdegen closed 6 months ago

aidanheerdegen commented 2 years ago

Add model driver for CABLE model

http://climate-cms.wikis.unsw.edu.au/CABLE

Closes #313

pep8speaks commented 2 years ago

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

Line 26:1: W293 blank line contains whitespace Line 61:80: E501 line too long (86 > 79 characters) Line 131:80: E501 line too long (89 > 79 characters)

Line 26:80: E501 line too long (81 > 79 characters) Line 30:80: E501 line too long (82 > 79 characters) Line 47:80: E501 line too long (80 > 79 characters) Line 50:80: E501 line too long (88 > 79 characters)

Comment last updated at 2023-12-20 02:49:10 UTC
coveralls commented 2 years ago

Coverage Status

coverage: 47.415% (-0.3%) from 47.733% when pulling e437db35e08e6c2badba0977b66542d3228540c1 on issue-313 into 8089edb1e1f57bae13cbe7e710ce04921378e745 on master.

aidanheerdegen commented 2 years ago

@ccarouge I have implemented a basic templating scheme for the cable.nml file.

There is an example of it's use in the template branch at this repo:

https://github.com/aidanheerdegen/cable_test/tree/template

The basic idea is to use $year in the cable.nml file, and read the value of $year from the previous restart, e.g. archive/restart007/cable.res.yaml, which for this example looks like so:

year: 1904

It's still a bit rough. There is no way to start this process without a cable.res.yaml file existing, as I wasn't sure how to get the value of year otherwise.

Just looked, and I can see it is available in the restart.nc file, so that would be the next step, interrogate that file if there is no cable.res.yaml file.

aidanheerdegen commented 11 months ago

I got notice that a CircleCI build failed. This is out of date because @dougiesquire has updated the CI to use GitHub Workflows.

I/we should rebase this and force push to the repo to get it up to date with those changes. Note that there is a conflict in payu/models/__init__.py which needs to be manually resolved.

Happy to go through the process next week @SeanBryan51 if that is helpful.

SeanBryan51 commented 8 months ago

In this payu experiment, we only have met forcing files from 1903 to 1909 - causing the model to crash when the year value exceeds 1909. Should we include logic to repeat the forcing once the model exceeds the duration of the forcing?

To implement this, I'm thinking we:

  1. Replace $year with $forcing_year in the templating scheme to distinguish the 'model' year from the 'forcing' year.
  2. Add an optional parameter forcing_year in cable.res.yaml which specifies the forcing year to use. If the forcing year is not specified, then the CABLE driver will use year as the forcing year.
  3. The CABLE driver will somehow need to know the 'min' and 'max' forcing year to cycle over. To do this, we could add an optional config file: cable.cycle.yaml (add to self.optional_config_files attribute) which specifies the min/max forcing year.

@aidanheerdegen do you see any issues with this?

aidanheerdegen commented 8 months ago

@SeanBryan51 and I had a IRL chat. I believe I suggested a setup script could be used to update links to forcing files to make them cycle correctly.

Is that still an workable option?

SeanBryan51 commented 7 months ago

I think I'm happy with the implementation of the driver. @aidanheerdegen or @jo-basevi are you happy to give this a review? Whenever you guys have the time.

aidanheerdegen commented 7 months ago

Happy to review .. so my first suggestion is to add some tests for your driver. Specifically the forcing logic, but if there are any other you can think of that might be useful add them to. There are some tests under payu/test/models/ so take a look for some prior art if that helps.

No-one wants to hear "write tests", but you know you should :) .. particularly the guy that just gave a skillshare on pytest!

;)

@jo-basevi has written some nice ones recently using parameterised decorators if you want inspiration.

SeanBryan51 commented 7 months ago

@aidanheerdegen I have added some tests for the forcing logic. I've tried to make the tests light weight by creating a separate helper function (the private methods in the Cable class had too much global state to write clean tests).

aidanheerdegen commented 6 months ago

@SeanBryan51 are you ready to merge this? Do you need another review?

SeanBryan51 commented 6 months ago

@aidanheerdegen after I clean up the commit history this should be all good to go in.

SeanBryan51 commented 6 months ago

Just doing some final checks to make sure things are working.

SeanBryan51 commented 6 months ago

Ready when you are @aidanheerdegen