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

Vertical grid shift: do not interpolate node values at nodata value (fixes #1002) #1004

Merged
merged 1 commit into from May 19, 2018

Conversation

@rouault
Copy link
Member

@rouault rouault commented May 19, 2018

I've made the choice of ignoring nodes that are the nodata value, but not completely fail the computation if at least one of the 4 nodes has a valid value (properly taking into account the weights of the valid nodes in the final value). I've no strong opinion if this is better than just failing if one of the value if nodata.

@rouault rouault force-pushed the rouault:fix_1002 branch from 4f987df to 66bfdb4 May 19, 2018
…ixes #1002)
@rouault rouault force-pushed the rouault:fix_1002 branch from 66bfdb4 to e9e9e72 May 19, 2018
@kbevers
Copy link
Member

@kbevers kbevers commented May 19, 2018

If I understand your implementation correct, it means that we are effectively extrapolating when determining the grid value where three of the nodes are NODATA, yes?

@rouault
Copy link
Member Author

@rouault rouault commented May 19, 2018

If I understand your implementation correct, it means that we are effectively extrapolating when determining the grid value where three of the nodes are NODATA, yes?

Yes, the final value in the case you mention will be the value of the only node != NODATA

@kbevers
Copy link
Member

@kbevers kbevers commented May 19, 2018

I am not a big fan of extrapolation here. It can be handled nicely but the implementation is a bit more complicated. We need at least two "good" nodes before we can do a meaningful interpolation. This results in the following cases for interpolation:

  1. 4/4 nodes: As previously. Never NODATA.
  2. 3/4 nodes: If the interpolation point is within the triangle made up of the three "good" nodes we can interpolate a value. Otherwise NODATA.
  3. 2/4 nodes: If the interpolation point is exactly on the line between the to "good" nodes we can interpolate a value. Otherwise NODATA.
  4. 1/4 nodes: Interpolation impossible. Always NODATA.

Wether this more complicated approach is worthwhile I am not sure. It is the correct way to do it I think, but seeing as the issue that sparked this has just recently surfaced after 15-20 years it may not be that big a deal :-)

@rouault
Copy link
Member Author

@rouault rouault commented May 19, 2018

Hum, your above suggestion sounds to me too complicated implementation-wise vs the value it adds. So I'd say either stick with what I implemented, or error out as soon as at least one node is NODATA.

By the way the interpolation I implemented here is what is done in GDAL in similar cases (which can be questioned indeed)

@kbevers
Copy link
Member

@kbevers kbevers commented May 19, 2018

I think this is fine. I might revisit the topic on a boring day in the future.

@kbevers kbevers merged commit d9a0cef into OSGeo:master May 19, 2018
3 checks passed
3 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.05%) to 77.604%
Details
@kbevers kbevers added this to the 5.1.0 milestone May 19, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.