Warper: Add TIE_STRATEGY warp option #11649
Conversation
| @@ -7676,7 +7711,25 @@ static void GWKAverageOrModeThread(void *pData) | |||
| { | |||
| const int nVal = | |||
| static_cast<int>(dfValueRealTmp); | |||
| if (++panVals[nVal + nBinsOffset] > nMaxVal) | |||
|
|
|||
| bool bValIsMax = | |||
There was a problem hiding this comment.
I know this isn't due to your changes, but the current variable naming is terrible and makes it hard to follow the algorithm. Would you mind renaming:
- panVals to something like panCount
- nMaxVal to nMaxCount
- iMaxInd to nModeValCandidate ?
- bValIsMax to bValIsMaxCount
I believe panRealSums could be removed, and just replaced by panCount shared between int and float implementations
There was a problem hiding this comment.
Yes, the variable names caused me some confusion when working on this. I'll try to improve them in a separate commit.
There was a problem hiding this comment.
Actually, I was surprised to see that "mode" resampling isn't accounting for pixel coverage fractions in the way that "sum" and "average" are. I'm planning to address this as I rename the variables.
There was a problem hiding this comment.
I'm planning to address this as I rename the variables.
yes, that would definitely make sense for "mode" as well
|
Looks reasonable to me, but if the ties are determined by the value of the samples, this implies that the classification is somehow sorted (like "CO2 rate [X-Y ppm]", "CO2 rate [Y-Z ppm]", etc ...) ? If so, wouldn't something more appropriate be to compute the average of the contributing samples and select the value in the contributing pixels which is the closest to that average? No need to generalize that to overview computation ? If so we could rather need new GRIORA_ or GRA_ enumerations rather than a warping option |
I'm using this with a land cover data. So while the values are not naturally sorted, I can reclassify them in a way that
I can imagine adding such an option, e.g. |
| { | ||
| iMaxVal = i; |
There was a problem hiding this comment.
@rouault probably not worth backporting, but this is doing extra work by not hitting break on pafRealVals[i] == fVal regardless of whether ++panRealSums[i] > panRealSums[iMaxVal]
I wasn't asking for it. Just thinking out loud and wondering if it would make some sense, but I'm not sure if there's a real use case. From your comment about your use case, it seems not |
What does this PR do?
Adds a
TIE_STRATEGYwarping option to provide some control over the destination pixel value during "mode" resampling when multiple source values appear at the same frequency. The current behavior of choosing the first value encountered can introduce a spatial bias (favoring values from the north/west) that is undesirable in some applications.What are related issues/pull requests?
Tasklist