Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement regridding during Dataset.load() #9

Merged
merged 14 commits into from
Jun 29, 2023
Merged

Conversation

BSchilperoort
Copy link
Contributor

@BSchilperoort BSchilperoort commented Jun 26, 2023

After a lot of messing around I've settled upon the following implementation:

  • A regridding implementation with flox and xarray (both available for all systems & on pypi).
  • An optional regridding method that uses xesmf (which requires esmpy/ESMF: only on linux, only on conda).

For most cases the two methods give basically the same result, although the custom regridding routine doesn't handle the following too well:

  • There is no weighted averaging performed: areas near the poles will incorrectly contribute more to the total average.
  • When the new resolution is close to the old resolution, an interpolation step is run first (to avoid anti-aliassing),and then the data is regridded to the intended resolution. This will lead to small inconsistencies with NaN thresholds.

How to review:

  • Download & ingest the eth canopy dataset.
  • Try out the .load method's resolution and regrid_method kwargs.

@BSchilperoort BSchilperoort marked this pull request as ready for review June 27, 2023 14:53
@BSchilperoort
Copy link
Contributor Author

Hi @geek-yang , this PR is ready for review!

You can ignore the failing windows tests for now, it's due to a pytest argument apparently not working (correctly) on Windows 🤪

@geek-yang
Copy link
Member

Hi @geek-yang , this PR is ready for review!

You can ignore the failing windows tests for now, it's due to a pytest argument apparently not working (correctly) on Windows 🤪

So for windows --ignore-glob='*xesmf*.py' this argument doesn't seem to work....(quite annoying...). A slightly nasty solution to bypass it, perhaps we can add the following to xesmf_regrid.py:

try:
    import xesmf
except ImportError:
    pass

This will not trigger the dependency check failure for pytest. Later when we need to add some unit tests for xesmf_regrid.py, we can put:

@pytest.mark.skipif('xesmf' not in sys.modules,
                    reason="requires the xesmf library")
def test_xesmf_function():
    ...

Copy link
Member

@geek-yang geek-yang left a comment

Choose a reason for hiding this comment

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

@BSchilperoort Super! Awesome solution for regridding! I've tested both methods for regridding (I need to get conda/mamba back for it.... 😒) and they all work perfectly. It would be nice to update the notebook as well (since now resolution and regrid_method are mandatory). But overall everything looks fantastic! You get a go 😜.

PS: if you plan to update the notebook, please also add equal_nan=True to the assertion of conversion (last cell), otherwise it might fail.

src/zampy/utils/xesmf_regrid.py Show resolved Hide resolved
src/zampy/datasets/eth_canopy_height.py Show resolved Hide resolved
@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@BSchilperoort
Copy link
Contributor Author

Thanks for the review, @geek-yang ! 😄

I fixed the pytest issue with --doctest-ignore-import-errors. I couldn't find that by searching the documentation, but by calling pytest --help. It seems that it's not very well known about in general, returning only 31 results on google.

@BSchilperoort BSchilperoort merged commit 54439a0 into main Jun 29, 2023
@BSchilperoort BSchilperoort deleted the implement-regrid branch June 29, 2023 08:36
@geek-yang
Copy link
Member

Thanks for the review, @geek-yang ! 😄

I fixed the pytest issue with --doctest-ignore-import-errors. I couldn't find that by searching the documentation, but by calling pytest --help. It seems that it's not very well known about in general, returning only 31 results on google.

Awesome! Yesterday I also did some googling but didn't find anything like this. It is quite nice that you checked --help 👍. I never expect --help to be helpful. Lesson for today haha 🤣 .

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.

2 participants