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

Return an error for /watershed API requests that fall on grid boundaries. #162

Open
corviday opened this issue Aug 20, 2020 · 11 comments
Open

Comments

@corviday
Copy link
Contributor

In the case where someone supplies a point to the watershed API that lies exactly on the boundary between cells, such as a whole number latitude or longitude, it's not clear which grid cell to use as the mouth of the watershed, which can make quite a large difference to the result.

@jameshiebert suggests a solution:

I'd propose that a simple resolution to this issue would consist of not allowing requests on cell boundaries. E.g. if the incoming request coordinate falls on a cell boundary, respond with an HTTP 400 Bad Request error (and an explanatory message).

@rod-glover
Copy link
Contributor

That seems somewhat user-unfriendly to me. It's an API not a UI, so we'd want to make sure that UIs don't ever confront the user with this error. Are we the only clients of this API?

@corviday
Copy link
Contributor Author

corviday commented Aug 24, 2020

Are we the only clients of this API?

MOTI might be using it in their asset management pilot project, which was the original purpose of the API. @sum1lim is using it for his analysis of RVIC's efficiency. We're not using it in any web service ourselves.

@jameshiebert
Copy link
Contributor

jameshiebert commented Aug 24, 2020

What's unfriendly about providing clear feedback about what is undefined behaviour (and potentially invalid results)?

We can certainly ensure that our UI doesn't do this, but the fact that we hit this edge case indicates that we need some guardrails, and that's exactly what an HTTP 400 is for.

@rod-glover
Copy link
Contributor

Put that way, nothing. The "not on a cell boundary" rule seemed arbitrary to me, and partly a product of our own software being inconsistent, but there is no real alternative except to provide inconsistent results. I withdraw the comment.

@jameshiebert
Copy link
Contributor

Sorry, why is it arbitrary? The watershed API answers questions for "all cells upstream of the input point". If the input point doesn't identify a single cell, how should one even answer that question? It's literally a boundary condition 😄

@rod-glover
Copy link
Contributor

Note to self: Do not comment on issues first thing Monday morning.

@rod-glover
Copy link
Contributor

OK, while we're amicably squabbling here, I'd propose the following rule: Points on cell boundaries are resolved as follows: A cell includes its northern and western boundary lines, and excludes its southern and eastern ones. That eliminates the "invalid location" condition in a simple way.

@corviday
Copy link
Contributor Author

corviday commented Aug 24, 2020

I'd propose the following rule: Points on cell boundaries are resolved as follows: A cell includes its northern and western boundary lines, and excludes its southern and eastern ones

Currently the way /watershed resolves the case of points that fall on a grid boundary and the way RVIC resolves the case of points that fall on a literal boundary do not match. This means that in the future someone might be able to calculate streamflow using RVIC with Osprey and then use the /watershed endpoint to get information on how big the watershed was, and those two pieces of data provided by us would not relate to eachother.

The example that alerted us to this issue involved RVIC asserting a "watershed" was two grid cells, and /watershed thinking it was 140 because they "rounded" the point the user gave, which was a whole number latitude and longitude, into different cells.

So what we're trying to avoid ultimately is a user requesting two pieces of information about a watershed from two different PCIC tools and getting contradictory responses. (Though in the case of a request on a cell boundary, is either answer correct?) So we can either:

  • have /watershed return an error if you ask for a point on a grid cell boundary (pros: easy for us, cons: confusing for user)
  • use a rule like your proposal, but do some detective work to see what rulings RVIC makes about grid boundaries, and reproduce them here (pros: maximum consistency between our tools, cons: more work for the programmer)

@rod-glover
Copy link
Contributor

Ah, I had a vague notion that consistency with RVIC was part of the issue, but didn't address it. That does sound important, and the boundary rule (like my proposal) sounds preferable to me if it is feasible.

@rod-glover
Copy link
Contributor

You could even make the rule selectable by a query param, so that it could be set for other cases (applications like RVIC to match) if necessary.

@jameshiebert
Copy link
Contributor

I'd definitely be careful about specifying one-and-only-one boundary behaviour (or at least being clear that that's what you're doing). Part of the reason that geometry engines have such a rich set of topological relationships is because geometric topology is complicated 😄

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

No branches or pull requests

3 participants