Issue 99 #120
Issue 99 #120
Conversation
… dask in structured arrays + fix for dx/dy output structured + add test with dask arrays (structured)
| "x": self._target.xbounds.midpoints, | ||
| } | ||
| ) | ||
| regridded = regridded.assign_coords(coords=self._target.coords) |
There was a problem hiding this comment.
Are the y coordinates still flipped in this implementation?
It is important to have y coordinates in descending order
I see you defined a new property coords, but I don't see it referring to midpoints, which I think is what we want?
There was a problem hiding this comment.
Added the coords property to the StructuredGrid- class. There it uses the actual grid properties directly and not the possible flipped midpoints. This implementation was needed to get the right dx/dy from the target-grid.
There was a problem hiding this comment.
Sounds good, just to double check: This doesn't result in grids which are flipped from descending y to ascending y coordinates?
There was a problem hiding this comment.
No it does not indeed. Only the midpoints arrays are fliped for computing the proper weights. The original coords are untouched and now used. I double checked myself + it is also tested in the testbench
xugrid/regrid/regridder.py
Outdated
| @@ -155,17 +165,16 @@ def _regrid_array(self, source): | |||
| chunks=chunks, | |||
| meta=np.array((), dtype=source.dtype), | |||
| ) | |||
| out = out.compute().reshape(out_shape) | |||
There was a problem hiding this comment.
Mind that the call to compute results that the results are loaded into memory. Not sure if that fits each use case. For now it is fine, we can optimize this later. Perhaps add a #TODO comment to later investigate this?
There was a problem hiding this comment.
yes that is possible suboptimal. I tried to use the dask.array.reshape() first, but that does not work for splitting up dimensions unevenly (https://docs.dask.org/en/stable/_modules/dask/array/reshape.html). So I used there suggestion to compute first. But definitely a TODO.
There was a problem hiding this comment.
Ah thanks for the info, a proper struggle with reshaping! We can resolve this conversation when the TODO is added, and I think we can make an issue for this!
|
|
||
| # E.g.: sizes of ("time", "layer") + ("y", "x") | ||
| out_shape = first_dims_shape + self._target.shape | ||
| return out.reshape(out_shape) |
There was a problem hiding this comment.
Why are you calling the reshape in the if else tree above, instead of at the end like it was? The original version appears to be cleaner to me, as reshape only has to be mentioned once, instead of twice after your alterations.
There was a problem hiding this comment.
Because the reshape does not work for a dask-array. For the dask-array I first used the dask.array.reshape(), which gives the result that we want. For the current implementation (see reply above) I could move it back indeed.
There was a problem hiding this comment.
Ah an understandable struggle with dask.array.reshape :-) I would prefer if you moved these lines back down because we can leave the code as it was now that the call to dask.array.compute was introduced.
…r defining blocks of extra dimension(s), use .shape for the topology dimension.
No description provided.