put feature feature corr behind flag (#479)

Author: leahmcguire

core/src/main/scala/com/salesforce/op/dsl/RichNumericFeature.scala

@@ -33,7 +33,7 @@ package com.salesforce.op.dsl
 import com.salesforce.op.features.FeatureLike
 import com.salesforce.op.features.types._
 import com.salesforce.op.stages.impl.feature._
-import com.salesforce.op.stages.impl.preparators.{CorrelationExclusion, CorrelationType, SanityChecker}
+import com.salesforce.op.stages.impl.preparators.{CorrelationExclusion, CorrelationLevel, CorrelationType, SanityChecker}
 import com.salesforce.op.stages.impl.regression.IsotonicRegressionCalibrator
 
 import scala.language.postfixOps
@@ -483,7 +483,7 @@ trait RichNumericFeature {
       protectTextSharedHash: Boolean = SanityChecker.ProtectTextSharedHash,
       maxRuleConfidence: Double = SanityChecker.MaxRuleConfidence,
       minRequiredRuleSupport: Double = SanityChecker.MinRequiredRuleSupport,
-      featureLabelCorrOnly: Boolean = SanityChecker.FeatureLabelCorrOnly,
+      featureFeatureCorrLevel: CorrelationLevel = SanityChecker.FeatureFeatureCorrLevel,
       correlationExclusion: CorrelationExclusion = SanityChecker.CorrelationExclusionDefault,
       categoricalLabel: Option[Boolean] = None
     ): FeatureLike[OPVector] = {
@@ -504,7 +504,7 @@ trait RichNumericFeature {
         .setProtectTextSharedHash(protectTextSharedHash)
         .setMaxRuleConfidence(maxRuleConfidence)
         .setMinRequiredRuleSupport(minRequiredRuleSupport)
-        .setFeatureLabelCorrOnly(featureLabelCorrOnly)
+        .setFeatureFeatureCorrLevel(featureFeatureCorrLevel)
         .setCorrelationExclusion(correlationExclusion)
         .setInput(f, featureVector)
 

core/src/main/scala/com/salesforce/op/stages/impl/preparators/SanityChecker.scala

@@ -177,13 +177,21 @@ trait SanityCheckerParams extends DerivedFeatureFilterParams {
   def setMinRequiredRuleSupport(value: Double): this.type = set(minRequiredRuleSupport, value)
   def getMinRequiredRuleSupport: Double = $(minRequiredRuleSupport)
 
-  final val featureLabelCorrOnly = new BooleanParam(
-    parent = this, name = "featureLabelCorrOnly",
-    doc = "If true, then only calculate the correlations between the features and the label. Otherwise, calculate " +
-      "the entire correlation matrix, which includes all feature-feature correlations."
+  final val featureFeatureCorrLevel = new Param[String](
+    parent = this, name = "featureFeatureCorrOnly",
+    doc = "This setting determines feature-feature correlation computations. Levels are: Off, Computed, Stored"
   )
-  def setFeatureLabelCorrOnly(value: Boolean): this.type = set(featureLabelCorrOnly, value)
-  def getFeatureLabelCorrOnly: Boolean = $(featureLabelCorrOnly)
+  def setFeatureFeatureCorrLevel(value: CorrelationLevel): this.type = set(featureFeatureCorrLevel, value.entryName)
+  def getFeatureFeatureCorrLevel: CorrelationLevel = CorrelationLevel.withName($(featureFeatureCorrLevel))
+
+  @deprecated("this setting is overridden by featureFeatureCorrLevel", "0.7.0")
+  def setFeatureLabelCorrOnly(value: Boolean): this.type = {
+    if (value) set(featureFeatureCorrLevel, CorrelationLevel.Off.entryName)
+    else set(featureFeatureCorrLevel, CorrelationLevel.Computed.entryName)
+  }
+
+  @deprecated("this setting is overridden by featureFeatureCorrLevel", "0.7.0")
+  def getFeatureLabelCorrOnly: Boolean = $(featureFeatureCorrLevel) == CorrelationLevel.Off.entryName
 
   final val correlationExclusion: Param[String] = new Param[String](this, "correlationExclusion",
     "Setting for what categories of feature vector columns to exclude from the correlation calculation",
@@ -208,7 +216,7 @@ trait SanityCheckerParams extends DerivedFeatureFilterParams {
     correlationType -> SanityChecker.CorrelationTypeDefault.entryName,
     maxRuleConfidence -> SanityChecker.MaxRuleConfidence,
     minRequiredRuleSupport -> SanityChecker.MinRequiredRuleSupport,
-    featureLabelCorrOnly -> SanityChecker.FeatureLabelCorrOnly,
+    featureFeatureCorrLevel -> SanityChecker.FeatureFeatureCorrLevel.entryName,
     correlationExclusion -> SanityChecker.CorrelationExclusionDefault.entryName
   )
 }
@@ -453,7 +461,7 @@ class SanityChecker(uid: String = UID[SanityChecker])
     else ((0 until featureSize + 1).toArray, vectorRows)
     val numCorrIndices = corrIndices.length
 
-    val (corrMatrix, corrsWithLabel) = if ($(featureLabelCorrOnly)) {
+    val (corrMatrix, corrsWithLabel) = if ($(featureFeatureCorrLevel) == CorrelationLevel.Off.entryName) {
       None -> OpStatistics.computeCorrelationsWithLabel(vectorRowsForCorr, colStats, count)
     }
     else {
@@ -513,7 +521,8 @@ class SanityChecker(uid: String = UID[SanityChecker])
       colStats = colStats,
       names = featureNames :+ in1.name,
       correlationType = CorrelationType.withNameInsensitive(corrType),
-      sample = sampleFraction
+      sample = sampleFraction,
+      keepFeatureFeature = getFeatureFeatureCorrLevel
     )
     setMetadata(outputMeta.toMetadata.withSummaryMetadata(summary.toMetadata()))
 
@@ -565,7 +574,7 @@ object SanityChecker {
   // These settings will make the maxRuleConfidence check off by default
   val MaxRuleConfidence = 1.0
   val MinRequiredRuleSupport = 1.0
-  val FeatureLabelCorrOnly = false
+  val FeatureFeatureCorrLevel = CorrelationLevel.Computed
   val CorrelationExclusionDefault = CorrelationExclusion.NoExclusion
 
   def SampleSeed: Long = util.Random.nextLong() // scalastyle:off method.name
@@ -620,3 +629,28 @@ object CorrelationExclusion extends Enum[CorrelationExclusion] {
    */
   case object HashedText extends CorrelationExclusion
 }
+
+
+/**
+ * Settings for feature - feature correlations
+ */
+sealed trait CorrelationLevel extends EnumEntry with Serializable
+
+object CorrelationLevel extends Enum[CorrelationLevel] {
+  val values: Seq[CorrelationLevel] = findValues
+
+  /**
+   * Feature-feature correlations are off
+   */
+  case object Off extends CorrelationLevel
+
+  /**
+   * Feature-feature correlations computed for feature exclusion
+   */
+  case object Computed extends CorrelationLevel
+
+  /**
+   * Feature-feature correlations stored in metadata
+   */
+  case object Stored extends CorrelationLevel
+}

core/src/main/scala/com/salesforce/op/stages/impl/preparators/SanityCheckerMetadata.scala

@@ -89,11 +89,17 @@ case class SanityCheckerSummary
     colStats: MultivariateStatisticalSummary,
     names: Seq[String],
     correlationType: CorrelationType,
-    sample: Double
+    sample: Double,
+    keepFeatureFeature: CorrelationLevel
   ) {
     this(
       correlations = new Correlations(
-        stats.filter(s => s.corrLabel.isDefined).map(s => (s.name, s.corrLabel.get, s.featureCorrs)),
+        stats.filter(s => s.corrLabel.isDefined).map { s =>
+          keepFeatureFeature match {
+            case CorrelationLevel.Stored => (s.name, s.corrLabel.get, s.featureCorrs)
+            case _ => (s.name, s.corrLabel.get, Seq.empty)
+          }
+        },
         correlationType
       ),
       dropped = dropped,
@@ -288,7 +294,7 @@ case class Correlations
   def this(corrs: Seq[(String, Double, Seq[Double])], corrType: CorrelationType) = this(
     featuresIn = corrs.map(_._1),
     valuesWithLabel = corrs.map(_._2),
-    valuesWithFeatures = corrs.map(_._3),
+    valuesWithFeatures = if (corrs.flatMap(_._3).isEmpty) Seq.empty else corrs.map(_._3),
     corrType = corrType
   )
 

core/src/test/scala/com/salesforce/op/stages/impl/preparators/SanityCheckerTest.scala

@@ -532,7 +532,7 @@ class SanityCheckerTest extends OpEstimatorSpec[OPVector, BinaryModel[RealNN, OP
       featuresToDrop, featuresWithNaNCorr, featuresIngore)
   }
 
-  it should "only calculate correlations between feature and the label if requested" in {
+  it should "only store correlations between feature and the label if requested" in {
     val smartMapVectorized = new SmartTextMapVectorizer[TextMap]()
       .setMaxCardinality(2).setNumFeatures(8).setMinSupport(1).setTopK(2).setPrependFeatureName(true)
       .setHashSpaceStrategy(HashSpaceStrategy.Shared)
@@ -544,7 +544,7 @@ class SanityCheckerTest extends OpEstimatorSpec[OPVector, BinaryModel[RealNN, OP
       .setRemoveBadFeatures(true)
       .setRemoveFeatureGroup(true)
       .setProtectTextSharedHash(true)
-      .setFeatureLabelCorrOnly(true)
+      .setFeatureFeatureCorrLevel(CorrelationLevel.Stored)
       .setMinCorrelation(0.0)
       .setMaxCorrelation(0.8)
       .setMaxCramersV(0.8)
@@ -564,7 +564,7 @@ class SanityCheckerTest extends OpEstimatorSpec[OPVector, BinaryModel[RealNN, OP
 
     val expectedFeatNames = featuresWithCorr ++ featuresWithNaNCorr
     validateTransformerOutput(checkedFeatures.name, transformed, expectedFeatNames,
-      featuresToDrop, featuresWithNaNCorr)
+      featuresToDrop, featuresWithNaNCorr, hasFeatureFeature = true)
   }
 
   it should "not fail when calculating feature-label correlations on a 5k element feature vector" in {
@@ -584,7 +584,7 @@ class SanityCheckerTest extends OpEstimatorSpec[OPVector, BinaryModel[RealNN, OP
       .setRemoveBadFeatures(false)
       .setRemoveFeatureGroup(true)
       .setProtectTextSharedHash(true)
-      .setFeatureLabelCorrOnly(true)
+      .setFeatureFeatureCorrLevel(CorrelationLevel.Off)
       .setMinVariance(-0.1)
       .setMinCorrelation(0.0)
       .setMaxCorrelation(0.8)
@@ -611,7 +611,7 @@ class SanityCheckerTest extends OpEstimatorSpec[OPVector, BinaryModel[RealNN, OP
 
     val expectedFeatNames = featuresWithCorr ++ featuresWithNaNCorr
     validateTransformerOutput(checkedFeatures.name, transformed, expectedFeatNames,
-      featuresToDrop, featuresWithNaNCorr)
+      featuresToDrop, featuresWithNaNCorr, hasFeatureFeature = false)
   }
 
   it should "not fail when maps have the same keys" in {
@@ -666,7 +666,8 @@ class SanityCheckerTest extends OpEstimatorSpec[OPVector, BinaryModel[RealNN, OP
     expectedFeatNames: Seq[String],
     expectedFeaturesToDrop: Seq[String],
     expectedCorrFeatNamesIsNan: Seq[String],
-    ignoredNames: Seq[String] = Seq.empty
+    ignoredNames: Seq[String] = Seq.empty,
+    hasFeatureFeature: Boolean = false
   ): Unit = {
     transformedData.select(outputColName).collect().foreach { case Row(features: Vector) =>
       features.toArray.length equals
@@ -682,6 +683,7 @@ class SanityCheckerTest extends OpEstimatorSpec[OPVector, BinaryModel[RealNN, OP
     } should contain theSameElementsAs expectedCorrFeatNamesIsNan
     summary.correlations.featuresIn should contain theSameElementsAs expectedFeatNames.diff(ignoredNames)
     summary.dropped should contain theSameElementsAs expectedFeaturesToDrop
+    summary.correlations.valuesWithFeatures.nonEmpty shouldEqual hasFeatureFeature
   }
 
   private def getMetadata(outputColName: String, transformedData: DataFrame): Metadata = {