Repository: deepjavalibrary/djl
Pull request #1723 (2022-06-17 21:23:15)

Ensure PreparedOneHotStringFeaturizer encodes categorical mappings co…
Ensure PreparedOneHotStringFeaturizer encodes categorical mappings consistently across dataset usages

## Description ##

For Tabular datasets, the PreparedOneHotStringFeaturizer currently builds a list of values for a feature as we [iterate through the records](https://github.com/deepjavalibrary/djl/blob/master/basicdataset/src/main/java/ai/djl/basicdataset/tabular/TabularDataset.java#L92-L94). Since we build the OHE mapping [based on the order the values are read](https://github.com/deepjavalibrary/djl/blob/master/basicdataset/src/main/java/ai/djl/basicdataset/tabular/utils/Featurizers.java#L171-L179), if the order of values read differs across datasets (train/test/val) then the OHE mappings differ across usages where the same model would be used.

To fix this, we should sort the values (how we sort doesn't matter as long as it is consistent across dataset usages) before constructing the OHE mappings. This does assume that the the values of a certain feature we want to OHE are the same and appear at least once across all dataset usages, which I think is a fair assumption to make. Featurizers should not be concerned with whether the dataset we are reading from has been constructed appropriately or not.

I updated the MovieLens100k dataset as part of this change since I noticed this issue while trying to create an example using this dataset.

## Example##

As an example, consider the following where we want to OHE the State feature:
```
--Train Data--
|row#|State|...
|1|CA|...
|2|CO|...
|3|NY|...

--Test Data--
|row#|State|...
|1|CO|...
|2|NY|...
|3|CA|...
```
The training data is prepared such that the OHE values are `CA=[1,0,0]`, `CO=[0,1,0]`, `NY=[0,0,1]` and the model we train will learn/assume those mappings apply to the test set we want to evaluate on. But in testing, we would prepare the OHE values as `CA=[0,0,1]`, `CO=[1,0,0]`, `NY=[0,1,0]` but the model wouldn't know that these mappings have changed.


codecov-commenter: # [Codecov](https://codecov.io/gh/deepjavalibrary/djl/pull/1723?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=deepjavalibrary) Report
> Merging [#1723](https://codecov.io/gh/deepjavalibrary/djl/pull/1723?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=deepjavalibrary) (80b01de) into [master](https://codecov.io/gh/deepjavalibrary/djl/commit/bb5073f7ffc33f1883621fc1783dbb01e70b418f?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=deepjavalibrary) (bb5073f) will **decrease** coverage by `1.56%`.
> The diff coverage is `70.73%`.

```diff
@@             Coverage Diff              @@
##             master    #1723      +/-   ##
============================================
- Coverage     72.08%   70.52%   -1.57%     
- Complexity     5126     5534     +408     
============================================
  Files           473      525      +52     
  Lines         21970    24618    +2648     
  Branches       2351     2662     +311     
============================================
+ Hits          15838    17361    +1523     
- Misses         4925     5934    +1009     
- Partials       1207     1323     +116     
```


| [Impacted Files](https://codecov.io/gh/deepjavalibrary/djl/pull/1723?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=deepjavalibrary) | Coverage Δ | |
|---|---|---|
| [api/src/main/java/ai/djl/modality/cv/Image.java](https://codecov.io/gh/deepjavalibrary/djl/pull/1723/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=deepjavalibrary#diff-YXBpL3NyYy9tYWluL2phdmEvYWkvZGpsL21vZGFsaXR5L2N2L0ltYWdlLmphdmE=) | `69.23% <ø> (-4.11%)` | :arrow_down: |
| [...rc/main/java/ai/djl/modality/cv/output/Joints.java](https://codecov.io/gh/deepjavalibrary/djl/pull/1723/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=deepjavalibrary#diff-YXBpL3NyYy9tYWluL2phdmEvYWkvZGpsL21vZGFsaXR5L2N2L291dHB1dC9Kb2ludHMuamF2YQ==) | `71.42% <ø> (ø)` | |
| [.../main/java/ai/djl/modality/cv/output/Landmark.java](https://codecov.io/gh/deepjavalibrary/djl/pull/1723/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=deepjavalibrary#diff-YXBpL3NyYy9tYWluL2phdmEvYWkvZGpsL21vZGFsaXR5L2N2L291dHB1dC9MYW5kbWFyay5qYXZh) | `100.00% <ø> (ø)` | |
| [...main/java/ai/djl/modality/cv/output/Rectangle.java](https://codecov.io/gh/deepjavalibrary/djl/pull/1723/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=deepjavalibrary#diff-YXBpL3NyYy9tYWluL2phdmEvYWkvZGpsL21vZGFsaXR5L2N2L291dHB1dC9SZWN0YW5nbGUuamF2YQ==) | `72.41% <0.00%> (ø)` | |
| [...i/djl/modality/cv/translator/BigGANTranslator.java](https://codecov.io/gh/deepjavalibrary/djl/pull/1723/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=deepjavalibrary#diff-YXBpL3NyYy9tYWluL2phdmEvYWkvZGpsL21vZGFsaXR5L2N2L3RyYW5zbGF0b3IvQmlnR0FOVHJhbnNsYXRvci5qYXZh) | `21.42% <ø> (-5.24%)` | :arrow_down: |
| [...odality/cv/translator/BigGANTranslatorFactory.java](https://codecov.io/gh/deepjavalibrary/djl/pull/1723/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=deepjavalibrary#diff-YXBpL3NyYy9tYWluL2phdmEvYWkvZGpsL21vZGFsaXR5L2N2L3RyYW5zbGF0b3IvQmlnR0FOVHJhbnNsYXRvckZhY3RvcnkuamF2YQ==) | `33.33% <0.00%> (+8.33%)` | :arrow_up: |
| [...nslator/InstanceSegmentationTranslatorFactory.java](https://codecov.io/gh/deepjavalibrary/djl/pull/1723/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=deepjavalibrary#diff-YXBpL3NyYy9tYWluL2phdmEvYWkvZGpsL21vZGFsaXR5L2N2L3RyYW5zbGF0b3IvSW5zdGFuY2VTZWdtZW50YXRpb25UcmFuc2xhdG9yRmFjdG9yeS5qYXZh) | `14.28% <0.00%> (-3.90%)` | :arrow_down: |
| [.../ai/djl/modality/cv/translator/YoloTranslator.java](https://codecov.io/gh/deepjavalibrary/djl/pull/1723/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=deepjavalibrary#diff-YXBpL3NyYy9tYWluL2phdmEvYWkvZGpsL21vZGFsaXR5L2N2L3RyYW5zbGF0b3IvWW9sb1RyYW5zbGF0b3IuamF2YQ==) | `8.33% <0.00%> (-0.50%)` | :arrow_down: |
| [.../modality/cv/translator/YoloTranslatorFactory.java](https://codecov.io/gh/deepjavalibrary/djl/pull/1723/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=deepjavalibrary#diff-YXBpL3NyYy9tYWluL2phdmEvYWkvZGpsL21vZGFsaXR5L2N2L3RyYW5zbGF0b3IvWW9sb1RyYW5zbGF0b3JGYWN0b3J5LmphdmE=) | `8.33% <0.00%> (-1.67%)` | :arrow_down: |
| [...i/djl/modality/cv/translator/YoloV5Translator.java](https://codecov.io/gh/deepjavalibrary/djl/pull/1723/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=deepjavalibrary#diff-YXBpL3NyYy9tYWluL2phdmEvYWkvZGpsL21vZGFsaXR5L2N2L3RyYW5zbGF0b3IvWW9sb1Y1VHJhbnNsYXRvci5qYXZh) | `5.38% <0.00%> (-0.31%)` | :arrow_down: |
| ... and [289 more](https://codecov.io/gh/deepjavalibrary/djl/pull/1723/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=deepjavalibrary) | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/deepjavalibrary/djl/pull/1723?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=deepjavalibrary).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=deepjavalibrary)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/deepjavalibrary/djl/pull/1723?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=deepjavalibrary). Last update [985390b...80b01de](https://codecov.io/gh/deepjavalibrary/djl/pull/1723?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=deepjavalibrary). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=deepjavalibrary).


Patch:
diff --git a/basicdataset/src/main/java/ai/djl/basicdataset/tabular/MovieLens100k.java b/basicdataset/src/main/java/ai/djl/basicdataset/tabular/MovieLens100k.java
index e1d53c367..0461a36c5 100644
--- a/basicdataset/src/main/java/ai/djl/basicdataset/tabular/MovieLens100k.java
+++ b/basicdataset/src/main/java/ai/djl/basicdataset/tabular/MovieLens100k.java
@@ -332,7 +332,7 @@ public final class MovieLens100k extends CsvDataset {
                 featureArray.forEach(feature -> addFeature(feature));
             }
             if (labels.isEmpty()) {
-                addNumericLabel("rating");
+                addCategoricalLabel("rating", true);
             }
             return new MovieLens100k(this);
         }
diff --git a/basicdataset/src/main/java/ai/djl/basicdataset/tabular/utils/Featurizers.java b/basicdataset/src/main/java/ai/djl/basicdataset/tabular/utils/Featurizers.java
index 26a20d0d2..a0d55eb26 100644
--- a/basicdataset/src/main/java/ai/djl/basicdataset/tabular/utils/Featurizers.java
+++ b/basicdataset/src/main/java/ai/djl/basicdataset/tabular/utils/Featurizers.java
@@ -17,6 +17,7 @@ import java.time.format.DateTimeFormatter;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.TreeSet;
 import java.util.concurrent.ConcurrentHashMap;
 
 /** A utility class provides helper functions to create {@link Featurizer}. */
@@ -170,7 +171,8 @@ public final class Featurizers {
         @Override
         public void prepare(List<String> inputs) {
             map = new ConcurrentHashMap<>();
-            for (String input : inputs) {
+            TreeSet<String> uniqueInputs = new TreeSet<>(inputs);
+            for (String input : uniqueInputs) {
                 if (!map.containsKey(input)) {
                     map.put(input, map.size());
                 }


Test patch:
diff --git a/basicdataset/src/test/java/ai/djl/basicdataset/MovieLens100kTest.java b/basicdataset/src/test/java/ai/djl/basicdataset/MovieLens100kTest.java
index 9560ee5e1..c7fe710cc 100644
--- a/basicdataset/src/test/java/ai/djl/basicdataset/MovieLens100kTest.java
+++ b/basicdataset/src/test/java/ai/djl/basicdataset/MovieLens100kTest.java
@@ -66,12 +66,13 @@ public class MovieLens100kTest {
             Assert.assertEquals(
                     data.head().toFloatArray(),
                     new float[] {
-                        23.0f, 1.0f, 0.0f, 0.0f, 0.0f, 1.0f, 0.0f, 0.0f, 0.0f, 0.0f, 0.0f,
+                        23.0f, 0.0f, 1.0f, 0.0f, 0.0f, 0.0f, 0.0f, 0.0f, 0.0f, 0.0f, 0.0f,
                         0.0f, 0.0f, 0.0f, 0.0f, 0.0f, 0.0f, 0.0f, 0.0f, 0.0f, 0.0f, 0.0f,
-                        0.0f, 0.0f, 0.0f, 1.0f, 0.0f, 0.0f, 0.0f, 0.0f, 0.0f, 0.0f, 0.0f,
+                        0.0f, 1.0f, 0.0f, 1.0f, 0.0f, 0.0f, 0.0f, 0.0f, 0.0f, 0.0f, 0.0f,
                         0.0f, 0.0f, 0.0f, 0.0f, 1.0f, 1.0f, 0.0f, 1.0f, 0.0f, 0.0f,
                     });
-            Assert.assertEquals(labels.head().toFloatArray(), new float[] {5.0f});
+            Assert.assertEquals(
+                    labels.head().toFloatArray(), new float[] {0.0f, 0.0f, 0.0f, 0.0f, 1.0f});
 
             try (Trainer trainer = model.newTrainer(config)) {
                 Batch batch = trainer.iterateDataset(movieLens100k).iterator().next();