DataCutter-related fixes for multiclass

[gerashegalov]

Related issues
Fixes #245

• label sort by count is nondeterministic for labels with the same count
• data cutter label trimming in validation is not called prior to bestEstimator
• column metadata is not updated after trimming
• in DataCutter.estimate whereas filter before sort is good for performance it may not drop labels non-deterministically unlike filtering over a stable order.

Describe the proposed solution
Remove non-determinism

[codecov]

Codecov Report

Merging #263 into master will decrease coverage by 1.85%.
The diff coverage is 91.66%.

@@            Coverage Diff             @@
##           master     #263      +/-   ##
==========================================
- Coverage   86.61%   84.75%   -1.86%     
==========================================
  Files         315      315              
  Lines       10345    10369      +24     
  Branches      346      563     +217     
==========================================
- Hits         8960     8788     -172     
- Misses       1385     1581     +196

Impacted Files	Coverage Δ
.../stages/impl/feature/OpStringIndexerNoFilter.scala	100% <100%> (ø)	⬆️
...sforce/op/stages/impl/selector/ModelSelector.scala	98.18% <100%> (+0.03%)	⬆️
.../salesforce/op/stages/impl/tuning/DataCutter.scala	91.54% <90%> (-4.11%)	⬇️
...sforce/op/stages/base/binary/BinaryEstimator.scala	0% <0%> (-100%)	⬇️
...stages/base/sequence/BinarySequenceEstimator.scala	0% <0%> (-100%)	⬇️
...orce/op/stages/base/ternary/TernaryEstimator.scala	0% <0%> (-100%)	⬇️
...m/salesforce/op/utils/spark/OpVectorMetadata.scala	0% <0%> (-85.46%)	⬇️
...sforce/op/utils/spark/OpVectorColumnMetadata.scala	0% <0%> (-76.09%)	⬇️
...m/salesforce/op/aggregators/ExtendedMultiset.scala	0% <0%> (-75%)	⬇️
...ages/impl/classification/OpXGBoostClassifier.scala	6.89% <0%> (-25.87%)	⬇️
... and 11 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6bb63ba...8333743. Read the comment docs.

[codecov]

Codecov Report

Merging #263 into master will decrease coverage by 3.77%.
The diff coverage is 99.02%.

@@            Coverage Diff             @@
##           master     #263      +/-   ##
==========================================
- Coverage   86.24%   82.47%   -3.78%     
==========================================
  Files         320      321       +1     
  Lines       10471    10539      +68     
  Branches      352      558     +206     
==========================================
- Hits         9031     8692     -339     
- Misses       1440     1847     +407

Impacted Files	Coverage Δ
...sforce/op/stages/impl/selector/ModelSelector.scala	98.24% <100%> (+0.09%)	⬆️
...org/apache/spark/ml/attribute/MetadataHelper.scala	100% <100%> (ø)
.../stages/impl/feature/OpStringIndexerNoFilter.scala	100% <100%> (ø)	⬆️
...alesforce/op/stages/impl/tuning/DataBalancer.scala	96.29% <100%> (+0.1%)	⬆️
...om/salesforce/op/stages/impl/tuning/Splitter.scala	97.72% <100%> (+0.58%)	⬆️
...e/op/stages/impl/selector/ModelSelectorNames.scala	100% <100%> (ø)	⬆️
...alesforce/op/stages/impl/tuning/DataSplitter.scala	66.66% <100%> (ø)	⬆️
.../salesforce/op/stages/impl/tuning/DataCutter.scala	96.84% <98.59%> (+1.18%)	⬆️
...alesforce/op/cli/gen/templates/SimpleProject.scala	0% <0%> (-100%)	⬇️
.../scala/com/salesforce/op/cli/gen/ProblemKind.scala	0% <0%> (-100%)	⬇️
... and 12 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9357a89...3d4198b. Read the comment docs.

[tovbinm]

@gerashegalov can you elaborate more on the proposed fixes? thanks.

[gerashegalov]

@tovbinm there are multiple issues.

One is the sort stability when there are multiple top labels with identical cardinality. We can solve this by adding the label as a secondary sort key. Similar issue with filtering labels for the minFraction prior to sorting. And #251 caused the data cutter omission to begin with

[tovbinm]

@gerashegalov please let us know when you want the PR reviewed.

[gerashegalov]

@tovbinm it's ready

[tovbinm]

Can you please add a new test case explicitly for label trimming issue?

[gerashegalov]

not ready for review yet, checkpointing WIP, sorting out test issues.

[gerashegalov]

marking as ready for review since all unit testing are passing

[leahmcguire]

Just remove the parts that were in there for testing and LGTM

[leahmcguire]

LGTM!!!!

[tovbinm]

🎆 🍾