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

Unpredictable result for first/last in aggregate_spatial / reduce_spatial #260

Closed
m-mohr opened this issue May 31, 2021 · 2 comments · Fixed by #261
Closed

Unpredictable result for first/last in aggregate_spatial / reduce_spatial #260

m-mohr opened this issue May 31, 2021 · 2 comments · Fixed by #261
Labels
help wanted Extra attention is needed must-have question Further information is requested
Milestone

Comments

@m-mohr
Copy link
Member

m-mohr commented May 31, 2021

While writing a proposal for reduce_spatial, it came up that I thought about "use cases" which compute a value depending on the order of the values. For example, passing a reducer such as first or last (or median) will not give predictable results depending on how the implementation "collects" the values from the underlying spatial areas/dimensions.

Has someone experiences from implementations how this could be handled and documented gracefully?

For now, I added text to the spec that documents a "naive" approach:

The list of values has an undefined order, therefore processes such as last() and first() that depend on the order of the values will lead to unpredictable results.

That makes it "documented", but not very useful for cases where you don't run e.g. "sort" before. What's your take on this?

@m-mohr m-mohr added help wanted Extra attention is needed question Further information is requested labels May 31, 2021
@m-mohr m-mohr added this to the 1.1.0 milestone May 31, 2021
m-mohr added a commit that referenced this issue May 31, 2021
m-mohr added a commit that referenced this issue May 31, 2021
@m-mohr m-mohr linked a pull request May 31, 2021 that will close this issue
@soxofaan
Copy link
Member

soxofaan commented Jun 1, 2021

I don't think this is worth to spend a lot of documentation on to be honest.
There is no intrinsic ordering for the combination of two dimensions (even if each dimension has a intrinsic order). I think it's enough to just warn the user to not assume some arbitrary order. Your proposal "naive" text is fine for me

m-mohr added a commit that referenced this issue Jun 4, 2021
reduce_spatial #226 and clarifications #260
@m-mohr
Copy link
Member Author

m-mohr commented Jun 4, 2021

Has been improved a bit in PR #261

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed must-have question Further information is requested
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants