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

Fix dependencies - sparse and numba #168

Merged
merged 4 commits into from
May 18, 2022
Merged

Fix dependencies - sparse and numba #168

merged 4 commits into from
May 18, 2022

Conversation

aulemahal
Copy link
Collaborator

Fixes #158

I tried sparse 0.7.0, 0.8.0, 0.9.1 and 0.10.0 (current is 0.13.0) and only 0.7.0 made the test suite fail. I thus assumed the minimum version to be 0.8.0.

Our explicit use of numba is very superficial, I didn't really check what the minimum version would be. We could simplify the code by pinning 0.55, but that's the last minor version, it felt too soon to pin only to remove half a line.

I also updated the pre-commit hooks (there was a bug with my version of black).

@aulemahal aulemahal requested a review from raphaeldussin May 13, 2022 20:24
@aulemahal
Copy link
Collaborator Author

aulemahal commented May 13, 2022

This is super weird, but the upstream failures are because of changes in xarray. I opened an issue : pydata/xarray#6607.

So, in test_frontend.py we have test datasets with 2D lon and lat. Then we have this function that transforms them into 1D lon and lat :

def ds_2d_to_1d(ds):
ds_temp = ds.reset_coords()
ds_1d = xr.merge([ds_temp['lon'][0, :], ds_temp['lat'][:, 0]])
ds_1d.coords['lon'] = ds_1d['lon']
ds_1d.coords['lat'] = ds_1d['lat']
return ds_1d

Then, in test_regrid_with_1d_grid_infer_bounds we call that function and rename x and y to lon and lat. This is a bit incomplete because if you inspect the dataset you realize that ds.lon.indexes is empty, even though ds.lon is a coordinate along a dimension of the same name. Then xesmf needs to cf_xarray to get the bounds. Taking the bounds involves the following operation: lon - lon.diff('lon'). Which fails because lon has no indexes, which are used to align the variables.

The thing is cf_xarray does call lon = lon.reset_coords(drop=True). I think this was planned to remove useless ancillary coordinates before computing the bounds. But it had the positive effect of kinda resetting lon so that the indexes were added. This is why the code worked up to the last release of xarray. A recent change in the master (I suspect "explicit indexes") broke that "workaround" and our test suite.

Ouf. That was a deep dive.

I'll push a fix for our code, I'm not sure if this is really a bug, or at least if it merits a real fix in xarray.

@aulemahal aulemahal requested a review from huard May 18, 2022 14:42
Copy link
Contributor

@huard huard left a comment

Choose a reason for hiding this comment

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

Thanks!

@huard huard merged commit 0069d6b into master May 18, 2022
@huard huard deleted the pin-sparse-numba branch May 18, 2022 17:12
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.

Dependency refinement sparse, numba
2 participants