New public release #5
Conversation
|
When we do this in the future, I would suggest having the first commit be a replacement rather than deletion of the code, so it's possible to see what files have changed. I don't think this is super important to review but I was curious to check it for any potential issues. |
| @@ -1,4 +1,4 @@ | |||
| __version__ = "0.1.0" | |||
| __version__ = "0.2.0" | |||
There was a problem hiding this comment.
We should update this in full-model also.
README.md
Outdated
| @@ -1,8 +1,8 @@ | |||
| # ACE: AI2 Climate Emulator | |||
| This repo contains the inference code accompanying "ACE: A fast, skillful learned global atmospheric model for climate prediction" ([arxiv:2310.02074](https://arxiv.org/abs/2310.02074)). | |||
|
|
|||
| ## DISCLAIMER | |||
| This is rapidly changing research software. We make no guarantees of maintaining backwards compatibility. | |||
| ## Documentation | |||
There was a problem hiding this comment.
Did you mean to remove the DISCLAIMER?
I would suggest not adding a header for a documentation link until we actually have said link. But we should probably include instructions for building the docs locally.
There was a problem hiding this comment.
It was intentional—we have a note in the docs about this being research software which I think is sufficient.
Sounds good, I'll remove the documentation section. I don't think we need to bother with local build instructions since we'll have the RTD page up soon.
| ## DISCLAIMER | ||
| This is rapidly changing research software. We make no guarantees of maintaining backwards compatibility. |
There was a problem hiding this comment.
Do we mean to remove this disclaimer?
There was a problem hiding this comment.
Yes, this was intentional. There is a similar warning in the docs.
There was a problem hiding this comment.
Thanks, @oliverwm1!
I was thinking about most of the configuration/data processing files in the scripts folder. Generally, I feel like it adds a lot of noise since there are a few important files for generating stats / ENSO metrics among them, but they are separated by folder so people could also probably just look for what they need.
Personally, since I think this will be one of our more widely seen and used repositories, I think it feels better just to have only relevant files distributed in a package rather than our internal processing. I would vote for cleaning up scripts. BUT, I realize this also adds an inconvenient divergence when merging into this repository as those scripts might be updated in full-model and create conflicts if they are deleted here.
Also, what's the plan for the documentation? Add that in after this merge? We could also consider linking the ACE colab notebook with a note at the beginning of the quickstart.
Makefile
Outdated
| @@ -1,11 +1,16 @@ | |||
| VERSION ?= $(shell git rev-parse --short HEAD) | |||
| IMAGE ?= fme | |||
| REGISTRY ?= registry.nersc.gov/m4492/ai2cm | |||
There was a problem hiding this comment.
Could probably remove the NERSC specific items for REGISTRY and push_shifter_image
There was a problem hiding this comment.
Yup thanks for reminder—thought I had done this.
I'm not too worried about conflicts. I did already delete some of the things in the
Yes, building the docs / hosting on RTD will be a follow on PR. Agreed, once this is merged to main and the colab notebook can reflect that, we can add a link. |
Update public code to latest.