creare-com / podpac

Pipeline for Observational Data Processing Analysis and Collaboration
https://podpac.org
Apache License 2.0
45 stars 6 forks source link

Affine coordinates #491

Closed jmilloy closed 2 years ago

jmilloy commented 2 years ago

AffineCoordinates are stacked spatial shaped coordinates that are parameterized by a geotransform. They replace RotatedCoordinates.

closes #363

jmilloy commented 2 years ago

@mpu-creare The test in test_units for the rotcoords roundtrip should pass before we consider this complete, but that doesn't cover all of the cases. Since it uses the source coordinates, it should short circuit, and we would need separate tests to hit the actual interpolators.

The AffineCoordinates are not complete. For example, they are assuming that the dims order is always lat-lon, though the lon-lat case is handled in a few places. It's just really complicated and needs real use-case in order to get it right, at least for me.

Another thing is that the interpolation machinery converts AffineCoordinates to StackedCoordinates in some places (basically just dropping the parameterization/geotransform). But ideally, we need to keep the geotransform whenever we can. For example, I had to add a case to the udrop method in this PR in order to preserve AffineCoordinates. We may need to something similar in other places. But a general solution is to implement the StackedCoordinates.is_affine property and StackedCoordinates.simplify method so that it converts shaped StackedCoordinates to unstacked ArrayCoordinates, unstacked UniformCoordinates, or AffineCoordinates depending on what is appropriate. I may spend some time on that part in parallel to your work on the interpolation. That is similar to the way that ArrayCoordinates1d have a is_uniform property and the simplify method returns UniformCoordinates when possible.

Related: unstacked spatial UniformCoordinates can also be handled using AffineCoordinates. Currently, podpac prefers unstacked UniformCoordinates. For example, the Coordinates.from_geotransform constructor checks for a non-rotated grid and uses unstacked UniformCoordinates instead of AffineCoordinates when possible. Ideally that won't be necessary once the AffineCoordinates are fully supported, and I'm curious whether UniformCoordinates will be better than AffineCoordinates (e.g. faster in the interpolation) or if we won't need to worry about which one is being used at some point.

jmilloy commented 2 years ago

Another thing is that the interpolation machinery converts AffineCoordinates to StackedCoordinates in some places

For example, the coordinates.transform in Interpolate._eval. I think the transform method calls simplify so getting that right would be a good fix.

mpu-creare commented 2 years ago

@jmilloy thanks for pointing all of that out. I'm trying to close out the ugly PR #493 then I'll focus on this one next.

jmilloy commented 2 years ago

@mpu-creare Building the geotransform directly made it really easy to implement AffineCoordinates indexing, thanks.

mpu-creare commented 2 years ago

@jmilloy I can see why you kicked this one over to me. The interpolation fix was small but TRICKY! Also, the from_xarray call became more complicated.

I'm going to implement the sub-selection and then call this one done.

mpu-creare commented 2 years ago

@jmilloy is there and ETA on this? I'd like to put together another release at some point.

jmilloy commented 2 years ago

Thanks for the prod, I think I can do it this afternoon.

mpu-creare commented 2 years ago

nice! Thanks @jmilloy