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

Interpolation ignores ctype #238

Closed
jmilloy opened this issue Apr 12, 2019 · 9 comments
Closed

Interpolation ignores ctype #238

jmilloy opened this issue Apr 12, 2019 · 9 comments
Labels
bug Something isn't working

Comments

@jmilloy
Copy link
Collaborator

jmilloy commented Apr 12, 2019

Description
The ctype ('left', 'right', 'midpoint', or 'point') for coordinates is not used anywhere.

Steps to Reproduce

import numpy as np
from podpac import Coordinates
from podpac.data import Array

data = np.random.random((100, 100))
nc = Coordinates([np.linspace(0, 10, 100), np.linspace(0, 10, 100)], dims=['lat', 'lon'], ctype='left')
node = Array(source=data, native_coordinates=nc)

c_left = Coordinates([np.linspace(1, 2, 20), np.linspace(1, 2, 20)], dims=['lat', 'lon'], ctype='left')
c_right = Coordinates([np.linspace(1, 2, 20), np.linspace(1, 2, 20)], dims=['lat', 'lon'], ctype='right')

o_left = node.eval(c_left)
o_right = node.eval(c_right)

Expected Behavior

o_left and o_right should be different, because they represent different locations in the data source.

Observed Behavior

o_left and o_right are the same.

Additional Notes

  • Possibly we can check ctype before intersecting or interpolating, the same way that we check crs. The transform method demonstrates that a similar as_ctype method could be easy to implement and integrate.
  • I'm not sure how the segment_lengths complicates this issue.
@jmilloy jmilloy added the bug Something isn't working label Apr 12, 2019
@jmilloy
Copy link
Collaborator Author

jmilloy commented Apr 12, 2019

@mpu-creare Have I thought this through correctly? And if so, what is the priority on this to you? Is it properly labeled a bug, or is it more of an enhancement to you? It goes right along with handling units and crs conversions -- does it belong the 1.0.0 release as a "feature complete" issue?

@mpu-creare
Copy link
Contributor

I'd label it as a bug... but it's probably more of a feature. You are correct, we haven't paid attention to it since we don't have a use case yet. We will very quickly begin having use-cases for point vs midpoint, but left and right will probably lag for quite a while.

@jmilloy
Copy link
Collaborator Author

jmilloy commented Apr 12, 2019

Maybe I can fix the "bug" aspect by adding a check when intersecting (maybe another when interpolating) than just raises an exception, and we can make a separate issue for the feature.

That could cause some issues pretty quickly because we default to 'midpoint' when we can (uniform coordinates or monotonic coordinates) and default to 'point' when we cannot (unordered arrays, singletons). I expect datasource native coordinates are commonly 'midpoint', and you will get an error if you evaluate a single point unless you specify ctype='midpoint' and provide a segment length. If it's easy enough to just implement, maybe we should just go for that instead?

@mpu-creare
Copy link
Contributor

I think we should address this... but I think this should be driven by an application otherwise we'll get it wrong. We have another project coming in that uses point data heavily, so we can take care of it there.

@jmilloy
Copy link
Collaborator Author

jmilloy commented Aug 12, 2019

@mpu-creare I still want to address the bug aspect of this, even if we don't implement it.

@jmilloy
Copy link
Collaborator Author

jmilloy commented Jan 24, 2020

We are going to remove the ctype/segments from Coordinates, so intersect will not need to handle that. The interpolation will need to be updated to handle the new DataSource segments. See #357.

@jmilloy jmilloy changed the title Interpolation and intersection ignores ctype Interpolation ~~and intersection~~ ignores ctype Jan 24, 2020
@jmilloy jmilloy changed the title Interpolation ~~and intersection~~ ignores ctype Interpolation ignores ctype Jan 24, 2020
@mpu-creare
Copy link
Contributor

@jmilloy should this be closed since it's no longer relevant? I think we need a new more specific issue.

@jmilloy
Copy link
Collaborator Author

jmilloy commented Aug 21, 2020

Yeah, I started to rewrite the issue, but really we need a feature issue, because the interpolation doesn't handle the datasource coordinate boundaries, but we (appropriately) throw a NotImplementedError if you try to use boundaries.

We should be able to do something like this, and get diferent results:

data = np.random.random((100, 100))
coords = Coordinates([np.linspace(0, 10, 100), np.linspace(0, 10, 100)], dims=['lat', 'lon'])
node1 = Array(source=data, coordinates=coords)
node2 = Array(source=data, coordinates=coords, boundary={'lat': [0, 1], 'lon': [0, 1]})
node3 = Array(source=data, coordinates=coords, boundary={'lat': [-1, 0], 'lon': [-1, 0]})

c = Coordinates([np.linspace(1, 2, 20), np.linspace(1, 2, 20)], dims=['lat', 'lon'])

o1 = node1.eval(c)
o2 = node2.eval(c)
o3 = node3.eval(c)

@jmilloy
Copy link
Collaborator Author

jmilloy commented Aug 21, 2020

See #422 422

@jmilloy jmilloy closed this as completed Aug 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants