Skip to content

Cice grid generation#6

Merged
anton-seaice merged 24 commits into
COSIMA:mainfrom
anton-seaice:cice_grid_generation
Apr 17, 2024
Merged

Cice grid generation#6
anton-seaice merged 24 commits into
COSIMA:mainfrom
anton-seaice:cice_grid_generation

Conversation

@anton-seaice
Copy link
Copy Markdown
Contributor

@anton-seaice anton-seaice commented Mar 20, 2024

Companion pull request to ACCESS-NRI/om3-scripts#8

  • fixes angle T
  • adds cf compliant variable names
  • add versioning to output

EDIT: This PR also adds tests. See COSIMA/om3-utils#5 for full scope of changes and review

@anton-seaice
Copy link
Copy Markdown
Contributor Author

I tried to run the tests ... it looks like they rely on some test data which doesn't exist anymore.

I get a 404 error trying to download from this link:

http://s3-ap-southeast-2.amazonaws.com/dp-drop/esmgrids/test/test_data.tar.gz

@anton-seaice
Copy link
Copy Markdown
Contributor Author

@aekiss & @ezhilsabareesh8 Can you review please?

@anton-seaice
Copy link
Copy Markdown
Contributor Author

@aekiss & @ezhilsabareesh8 Can you review please?

Bump :)

Copy link
Copy Markdown

@ezhilsabareesh8 ezhilsabareesh8 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Anton, it looks good to me and clear

Comment thread esmgrids/mom_grid.py Outdated
# The angle of rotation is a corner cell centres and applies to
# both t and u cells.
angle_t = angle_dx[2::2, 2::2]

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be add a comment why we are changing the index to 1::2, 1::2

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know how to write a concise comment to say it ... The rest of the file is uncommented so i just followed that style !

@micaeljtoliveira
Copy link
Copy Markdown
Contributor

@anton-seaice I suggest you drop the last three commits and rebase onto the latest main.

@dougiesquire
Copy link
Copy Markdown
Collaborator

@anton-seaice, I'm happy to do a review once you've brought in the relevant parts of COSIMA/om3-utils#5 (i.e. the additional provenance metadata and the tests)

@anton-seaice anton-seaice force-pushed the cice_grid_generation branch from e5791f1 to 9a1682f Compare April 12, 2024 04:23
@anton-seaice
Copy link
Copy Markdown
Contributor Author

@dougiesquire I think this is mostly done. Its a bit messy, which I will try and tidy up.

Comment thread README.md Outdated
Comment thread pyproject.toml Outdated
@anton-seaice anton-seaice force-pushed the cice_grid_generation branch from 12c0ad1 to f500eb7 Compare April 14, 2024 23:22
@dougiesquire
Copy link
Copy Markdown
Collaborator

@anton-seaice I'm not working today, but let me know when you're done making changes and I'll do a review first thing tomorrow

@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 15, 2024

Welcome to Codecov 🎉

Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests.

Thanks for integrating Codecov - We've got you covered ☂️

@micaeljtoliveira
Copy link
Copy Markdown
Contributor

micaeljtoliveira commented Apr 16, 2024

Yeah, hopefully it would never look like that in production files, since the last part indicates uncommitted changes (which we can never keep track of). I would hope that all production files would be created with tagged releases ( so something like 0.2.0), but even unreleased commits will be kept track of (e.g. 0.2.0.dev+g).

One could add a check to stop the code if the file is being generated with uncommitted changes and issue a warning if it's not a tagged release.

@dougiesquire
Copy link
Copy Markdown
Collaborator

One could add a check to stop the code if the file is being generated with uncommitted changes and issue a warning if it's not a tagged release.

Sure, I'll do that now

@dougiesquire
Copy link
Copy Markdown
Collaborator

@anton-seaice, one more PR from me. Then IMO we're ready for a final review and merge

Copy link
Copy Markdown
Collaborator

@dougiesquire dougiesquire left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thanks for this @anton-seaice - a big and important chunk of work

This was referenced Apr 17, 2024
@anton-seaice anton-seaice merged commit 1b6837d into COSIMA:main Apr 17, 2024
@aekiss
Copy link
Copy Markdown

aekiss commented Apr 17, 2024

Thanks @anton-seaice and everyone - great work!

anton-seaice added a commit to ACCESS-NRI/access-om3-configs that referenced this pull request May 14, 2024
anton-seaice added a commit to ACCESS-NRI/access-om3-configs that referenced this pull request May 14, 2024
This allows much stricter checking of angles in the ESMF files compared to the model grid files (per https://github.com/COSIMA/access-om3/issues/144)
anton-seaice added a commit to ACCESS-NRI/access-om3-configs that referenced this pull request May 16, 2024
This allows much stricter checking of angles in the ESMF files compared to the model grid files (per https://github.com/COSIMA/access-om3/issues/144)
anton-seaice added a commit to ACCESS-NRI/access-om3-configs that referenced this pull request May 16, 2024
This allows much stricter checking of angles in the ESMF files compared to the model grid files (per https://github.com/COSIMA/access-om3/issues/144)
anton-seaice added a commit to ACCESS-NRI/access-om3-configs that referenced this pull request May 16, 2024
This allows much stricter checking of angles in the ESMF files compared to the model grid files (per https://github.com/COSIMA/access-om3/issues/144)
anton-seaice added a commit to ACCESS-NRI/access-om3-configs that referenced this pull request May 17, 2024
This allows much stricter checking of angles in the ESMF files compared to the model grid files (per https://github.com/COSIMA/access-om3/issues/144)
anton-seaice added a commit to ACCESS-NRI/access-om3-configs that referenced this pull request May 17, 2024
This allows much stricter checking of angles in the ESMF files compared to the model grid files (per https://github.com/COSIMA/access-om3/issues/144)
anton-seaice added a commit to ACCESS-NRI/access-om3-wav-configs that referenced this pull request May 17, 2024
This allows much stricter checking of angles in the ESMF files compared to the model grid files (per https://github.com/COSIMA/access-om3/issues/144)
anton-seaice added a commit to ACCESS-NRI/access-om3-wav-configs that referenced this pull request May 17, 2024
This allows much stricter checking of angles in the ESMF files compared to the model grid files (per https://github.com/COSIMA/access-om3/issues/144)
anton-seaice added a commit to ACCESS-NRI/access-om3-wav-configs that referenced this pull request May 17, 2024
This allows much stricter checking of angles in the ESMF files compared to the model grid files (per https://github.com/COSIMA/access-om3/issues/144)
anton-seaice added a commit to ACCESS-NRI/access-om3-wav-configs that referenced this pull request May 17, 2024
This allows much stricter checking of angles in the ESMF files compared to the model grid files (per https://github.com/COSIMA/access-om3/issues/144)
anton-seaice added a commit to ACCESS-NRI/access-om3-wav-configs that referenced this pull request May 17, 2024
This allows much stricter checking of angles in the ESMF files compared to the model grid files (per https://github.com/COSIMA/access-om3/issues/144)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants