[SEDONA-475] Add RS_NormalizeAll #1221

prantogg · 2024-01-30T21:36:13Z

Did you read the Contributor Guide?

Yes, I have read Contributor Rules and Contributor Development Guide

Is this PR related to a JIRA ticket?

Yes, the URL of the associated JIRA ticket is https://issues.apache.org/jira/browse/SEDONA-475. The PR name follows the format [SEDONA-XXX] my subject.

What changes were proposed in this PR?

Adds RS_NormalizeAll, to normalize values in all bands of a raster between a given normalization range.
Key features -

Flexible Normalization Range: Optionally specify normalization range through minLim and maxLim. By default, the values are normalized to range [0, 255].
Handling Uniform Bands: For bands where all pixel values are identical, normalized values are set to lower limit of normalization range.
Handling NoDataValues: Allows defining noDataValue to be used for missing or invalid data in raster bands. By default, noDataValue is set to maxLim.
Optional Min/Max range of raster: Min and Max range of the input raster's pixel values can be specified via minValue and maxValue to skip computation of them during runtime.
Data Type Integrity: The function maintains the data type of raster values ensuring normalized values are cast back to the original raster's data type.
Normalization across bands: normalizeAcrossBands flag, when set, normalization is performed across all bands based on global min and max values. If false, each band is normalized individually based on its own min and max values.

How was this patch tested?

Passes new and existing tests

Did this PR include necessary documentation updates?

Yes, I am adding a new API. I am using the current SNAPSHOT version number in since vX.Y.Z format.

jiayuasu · 2024-01-31T08:38:45Z

common/src/main/java/org/apache/sedona/common/raster/MapAlgebra.java

+
+            // Normalize the band values
+            for (int i = 0; i < bandValues.length; i++) {
+                bandValues[i] = minLim + ((bandValues[i] - minValue) * (maxLim - minLim)) / (maxValue - minValue);


What will happen if all band values are the same. Then the denominator becomes zero and hence will lead to unexpected behavior.

What if all band values are zero?

What happens to the NoDataValue of a raster after normalization? @rbavery any experience?

After discussing with @rbavery, we should exclude NoDataValue when performing the normalization. Moreover, we should provide an optional parameter for users to set the new NoDataValue.

jiayuasu · 2024-01-31T08:54:57Z

common/src/test/java/org/apache/sedona/common/raster/MapAlgebraTest.java

+            double normalizedMax1 = Arrays.stream(normalizedBand1).max().getAsDouble();
+            double normalizedMin2 = Arrays.stream(normalizedBand2).min().getAsDouble();
+            double normalizedMax2 = Arrays.stream(normalizedBand2).max().getAsDouble();
+            double[] expected1 = {0.0, 17.0, 34.0, 51.0, 68.0, 85.0, 102.0, 119.0, 136.0, 153.0, 170.0, 187.0, 204.0, 221.0, 238.0, 255.0};


Please add tests for bands that have all same values.

Please add tests for rasters that have different pixel types. NormalizeAll is not supposed to change the pixel types. Moreover, the data truncation should happen as expected. If the input raster is integer, the normalized output raster should still have integer values, which means some decimal places derived from normalization will get truncated. One example is from MapAlgebra: https://sedona.apache.org/1.5.1/api/sql/Raster-map-algebra/#map-algebra

jiayuasu · 2024-01-31T18:17:45Z

common/src/main/java/org/apache/sedona/common/raster/MapAlgebra.java

+            // Find min and max values in the band
+            double minValue = Arrays.stream(bandValues).min().orElse(Double.NaN);
+            double maxValue = Arrays.stream(bandValues).max().orElse(Double.NaN);
+            System.out.println("minValue: "+minValue);


Please do not commit System.out.println

jiayuasu

In short, this function should have the following variants:

RS_NormalizeAll (raster: Raster)

RS_NormalizeAll (raster: Raster, minLim: Double, maxLim: Double)

RS_NormalizeAll (raster: Raster, minLim: Double, maxLim: Double, NoDataValue:Double)

RS_NormalizeAll (raster: Raster, minLim: Double, maxLim: Double, NoDataValue:Double, minValue:Double, maxValue:Double)

jiayuasu · 2024-01-31T18:39:04Z

common/src/main/java/org/apache/sedona/common/raster/MapAlgebra.java

+
+            if (minValue == maxValue) {
+                // If all values in the band are same - set middle value of range as default value.
+                double defaultValue = (minLim + maxLim) / 2.0;


If this happens, please choose minLim as the default value.

Please describe the behavior in the document.

Sure. Have described the current behavior in the PR description above.

common/src/main/java/org/apache/sedona/common/raster/MapAlgebra.java

jiayuasu · 2024-01-31T23:32:01Z

common/src/main/java/org/apache/sedona/common/raster/MapAlgebra.java

@@ -437,6 +437,90 @@ public static double[] normalize(double[] band) {
        return result;
    }

+    public static GridCoverage2D normalizeAll(GridCoverage2D rasterGeom) {
+        return normalizeAll(rasterGeom, 0d, 255d, -9999, -99999, -99999);


Please don't use magic numbers here. Can you use null?

jiayuasu · 2024-01-31T23:33:59Z

common/src/main/java/org/apache/sedona/common/raster/MapAlgebra.java

+            // Get the band values as an array
+            double[] bandValues = bandAsArray(rasterGeom, bandIndex);
+
+            // Find min and max values in the band, excluding NoDataValue


You should fetch the NoDataValue from each band. There are operators functions in RasterUtils can already do it. The NoDataValue of the input is to be used when storing in the raster band.

jiayuasu · 2024-01-31T23:34:25Z

common/src/main/java/org/apache/sedona/common/raster/MapAlgebra.java

+        }
+
+        // Unset minmaxFlag if minValue and maxValue are not given;
+        boolean minmaxFlag = minValue != -99999;


Please check null instead of a magic number.

jiayuasu · 2024-01-31T23:38:06Z

common/src/main/java/org/apache/sedona/common/raster/MapAlgebra.java

+                // Normalize the band values, setting NoDataValue to maxLim
+                for (int i = 0; i < bandValues.length; i++) {
+                    if (bandValues[i] == noDataValue) {
+                        bandValues[i] = maxLim;


If the band value equals to the no data value of a band, then you should set it as noDataValue from the input.

If the input noDataValue is not given, then set it to maxLim.

@jiayuasu setting noDataValue to the maxLim by default will corrupt real data values.

instead I think the default should be to scale real values from 0, 254 and set the noDataValue to 255 if it is not given.

Or, scale real values between 1-255, set noDataValue to 0 by default. this makes more sense to me, 0 is used as a noDataValue more often than 255.

* Implement RS_NormalizeAll * add IllegalArgumentException * Fix override issue * Handle same band values * Refactor NormalizeAll; Add tests * Update Documentation * Add flag argument: normalizeAcrossBands * fix lint * Optimize normalizeAll: remove redundant min/max calculations

prantogg added 3 commits January 30, 2024 16:15

Implement RS_NormalizeAll

16cef2e

add IllegalArgumentException

9359d95

Fix override issue

98b25ec

prantogg changed the title ~~[SEDONA-475] Add RS_NormalizeAll~~ [SEDONA-475][EWT-558] Add RS_NormalizeAll Jan 30, 2024

prantogg changed the title ~~[SEDONA-475][EWT-558] Add RS_NormalizeAll~~ [SEDONA-475] Add RS_NormalizeAll Jan 30, 2024

jiayuasu added improvement affect public APIs sedona-spark labels Jan 31, 2024

jiayuasu added this to the sedona-1.6.0 milestone Jan 31, 2024

jiayuasu requested changes Jan 31, 2024

View reviewed changes

Handle same band values

ca0c958

jiayuasu requested changes Jan 31, 2024

View reviewed changes

prantogg added 2 commits January 31, 2024 17:16

Refactor NormalizeAll; Add tests

3189914

Update Documentation

f0f3c5d

prantogg requested a review from jiayuasu January 31, 2024 23:20

jiayuasu requested changes Jan 31, 2024

View reviewed changes

common/src/main/java/org/apache/sedona/common/raster/MapAlgebra.java Outdated Show resolved Hide resolved

jiayuasu requested changes Jan 31, 2024

View reviewed changes

Add flag argument: normalizeAcrossBands

fe56116

prantogg requested a review from jiayuasu February 1, 2024 23:46

prantogg added 2 commits February 1, 2024 18:50

fix lint

fd1141e

Optimize normalizeAll: remove redundant min/max calculations

1882fea

jiayuasu approved these changes Feb 3, 2024

View reviewed changes

jiayuasu merged commit adebdac into apache:master Feb 3, 2024
47 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SEDONA-475] Add RS_NormalizeAll #1221

[SEDONA-475] Add RS_NormalizeAll #1221

prantogg commented Jan 30, 2024 •

edited

Loading

jiayuasu Jan 31, 2024

jiayuasu Jan 31, 2024

jiayuasu Jan 31, 2024

jiayuasu Jan 31, 2024

jiayuasu Jan 31, 2024

jiayuasu Jan 31, 2024

jiayuasu left a comment

jiayuasu Jan 31, 2024

prantogg Jan 31, 2024

jiayuasu Jan 31, 2024

jiayuasu Jan 31, 2024

jiayuasu Jan 31, 2024

jiayuasu Jan 31, 2024

rbavery Feb 7, 2024

[SEDONA-475] Add RS_NormalizeAll #1221

[SEDONA-475] Add RS_NormalizeAll #1221

Conversation

prantogg commented Jan 30, 2024 • edited Loading

Did you read the Contributor Guide?

Is this PR related to a JIRA ticket?

What changes were proposed in this PR?

How was this patch tested?

Did this PR include necessary documentation updates?

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jiayuasu left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

prantogg commented Jan 30, 2024 •

edited

Loading