NCAR / icar

The Intermediate Complexity Atmospheric Research model (ICAR)
MIT License
72 stars 53 forks source link

Restarted from enhancement to aggregation script and small fixes #185

Open scrasmussen opened 4 months ago

scrasmussen commented 4 months ago

TYPE: bugfix

KEYWORDS: precipitation, restarts, ideal tests, Python requirements

SOURCE: Soren Rasmussen, NCAR

DESCRIPTION OF CHANGES:

ISSUE: Fixes #180 and negative precip

TESTS CONDUCTED: Used the ideal testing files to do runs testing the new features in the aggregate script. Tested behavior of script with different restarted_from NetCDF values. Ran new aggregate script on old outputted files that had negative precip and it fixed the issue.

NOTES: Optional, as appropriate. Delete if not used. Included only once for new features requiring several merge cycles. Changes to default behavior are also note worthy.

Checklist

scrasmussen commented 4 months ago

Hi @gutmann, sorry for the slow changes, took three days of PTO so was a bit slow getting to this but the things we talked about should be encapsulated in these new commits. I changed the README to describe the additional Python package requirements.

I should note that the Bunch package is 12 years old and doesn't support Python 3. I can look at updating the make_domain.py script if you'd like.

gutmann commented 4 months ago

Thanks, if I recall correctly, "bunch" is a ~10 line python module... either it could be updated or we can just drop it. I suspect none of the scripts that are actively used anymore still use it. Let's just move all the scripts that do rely on it to some sort of deprecated archive folder for reference. I realize we could just delete them and they will still exist in git, but nobody would find them if we do that, and they could still be useful for someone to modify for future projects.

gutmann commented 4 months ago

Weird, looking at that bunch repo, it has a python3 compatibility, does that not work? Also, funny that the core 10-line functionality has now ballooned into 400+ lines of code with all the comments, yaml/json support, etc. Useful, but then with just 10-lines I knew what it was doing 🙃

scrasmussen commented 4 months ago

PR should be good to go, here is a summary of changes since there were a lot of things stuck into this one: