Skip to content

Feat/masks review#199

Merged
aaron-hopkinson merged 15 commits intofeat/masksfrom
feat/masks-review
Jan 22, 2026
Merged

Feat/masks review#199
aaron-hopkinson merged 15 commits intofeat/masksfrom
feat/masks-review

Conversation

@aaron-hopkinson
Copy link
Contributor

Implement outstanding review comments on #108

The name of the grid
id : str | list[float] | tuple[float, float]
The identifier of the grid, either a string like "o96" or a tuple/list
of two numbers (describing the resolution).
Copy link
Contributor Author

Choose a reason for hiding this comment

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

For info: renamed function and added more info to docstring

Copy link
Contributor

@yoel-zerah yoel-zerah Jan 22, 2026

Choose a reason for hiding this comment

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

id is a bad argument name, because it is a python built-in function, I propose grid_id.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed to grid_id. Thanks!

Copy link
Contributor

@yoel-zerah yoel-zerah left a comment

Choose a reason for hiding this comment

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

(From #108) In tests/test_regrid.py, in test_make_regrid_mask the size of the mask produced by global-on-lam-mask is checked against a reference provided in "anemoi-transform/filters/regrid/ea-over-rr-mask.npz".
Unfortunately, this reference mask has the wrong shape: the mask tested is generated by cropping ERA5 to CERRA (under 38k grid points), but the reference mask contains all of the original 542k ERA5 grid points.

  • How can we change the provided mask with another file ?
  • How do we generate a new reference file ? At the moment the only way I can generate a mask is with this feature being tested...

Should this be fixed or removed ?

@github-project-automation github-project-automation bot moved this from To be triaged to Under Review in Anemoi-dev Jan 21, 2026
@aaron-hopkinson
Copy link
Contributor Author

@yoel-zerah It should now be fixed (but doesn't require a code change)



@skip_if_offline
@pytest.mark.slow
Copy link
Contributor

Choose a reason for hiding this comment

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

where are we using this? as in do those run then at PR level, nightly or which is the frequency?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

By default we won't be running them in the CI, which is as the same as in anemoi-datasets. I run these locally before pushing up a PR (as with datasets)

Copy link
Contributor

@anaprietonem anaprietonem left a comment

Choose a reason for hiding this comment

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

Left some minor comments mostly for clarify

@anaprietonem anaprietonem self-requested a review January 22, 2026 12:57
Copy link
Contributor

@anaprietonem anaprietonem left a comment

Choose a reason for hiding this comment

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

Thanks for addressing my comment, LGTM

@aaron-hopkinson
Copy link
Contributor Author

Thank you!

@aaron-hopkinson aaron-hopkinson merged commit 7081756 into feat/masks Jan 22, 2026
66 of 69 checks passed
@aaron-hopkinson aaron-hopkinson deleted the feat/masks-review branch January 22, 2026 14:37
@github-project-automation github-project-automation bot moved this from Under Review to Done in Anemoi-dev Jan 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ATS Approval not needed dependencies Pull requests that update a dependency file tests

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants