[SPARK-18034] Upgrade to MiMa 0.1.11 to fix flakiness

[JoshRosen]

We should upgrade to the latest release of MiMa (0.1.11) in order to include a fix for a bug which led to flakiness in the MiMa checks (lightbend-labs/mima#115).

[SparkQA]

Test build #67284 has finished for PR 15571 at commit b68cead.

• This patch fails MiMa tests.
• This patch merges cleanly.
• This patch adds no public classes.

[JoshRosen]

Hmm, looks like MiMa gained some new checks in its latest release:

[error]  * abstract method aggregationDepth()org.apache.spark.ml.param.IntParam in trait org.apache.spark.ml.param.shared.HasAggregationDepth is inherited by class LogisticRegressionParams in current version.
[error]    filter with: ProblemFilters.exclude[InheritedNewAbstractMethodProblem]("org.apache.spark.ml.param.shared.HasAggregationDepth.aggregationDepth")
[error]  * abstract method getAggregationDepth()Int in trait org.apache.spark.ml.param.shared.HasAggregationDepth is inherited by class LogisticRegressionParams in current version.
[error]    filter with: ProblemFilters.exclude[InheritedNewAbstractMethodProblem]("org.apache.spark.ml.param.shared.HasAggregationDepth.getAggregationDepth")
[error]  * abstract synthetic method org$apache$spark$ml$param$shared$HasAggregationDepth$_setter_$aggregationDepth_=(org.apache.spark.ml.param.IntParam)Unit in trait org.apache.spark.ml.param.shared.HasAggregationDepth is inherited by class LogisticRegressionParams in current version.
[error]    filter with: ProblemFilters.exclude[InheritedNewAbstractMethodProblem]("org.apache.spark.ml.param.shared.HasAggregationDepth.org$apache$spark$ml$param$shared$HasAggregationDepth$_setter_$aggregationDepth_=")
[error]  * abstract method aggregationDepth()org.apache.spark.ml.param.IntParam in trait org.apache.spark.ml.param.shared.HasAggregationDepth is inherited by class LinearRegressionParams in current version.
[error]    filter with: ProblemFilters.exclude[InheritedNewAbstractMethodProblem]("org.apache.spark.ml.param.shared.HasAggregationDepth.aggregationDepth")
[error]  * abstract method getAggregationDepth()Int in trait org.apache.spark.ml.param.shared.HasAggregationDepth is inherited by class LinearRegressionParams in current version.
[error]    filter with: ProblemFilters.exclude[InheritedNewAbstractMethodProblem]("org.apache.spark.ml.param.shared.HasAggregationDepth.getAggregationDepth")
[error]  * abstract synthetic method org$apache$spark$ml$param$shared$HasAggregationDepth$_setter_$aggregationDepth_=(org.apache.spark.ml.param.IntParam)Unit in trait org.apache.spark.ml.param.shared.HasAggregationDepth is inherited by class LinearRegressionParams in current version.
[error]    filter with: ProblemFilters.exclude[InheritedNewAbstractMethodProblem]("org.apache.spark.ml.param.shared.HasAggregationDepth.org$apache$spark$ml$param$shared$HasAggregationDepth$_setter_$aggregationDepth_=")
[error]  * abstract method aggregationDepth()org.apache.spark.ml.param.IntParam in trait org.apache.spark.ml.param.shared.HasAggregationDepth is inherited by class AFTSurvivalRegressionParams in current version.
[error]    filter with: ProblemFilters.exclude[InheritedNewAbstractMethodProblem]("org.apache.spark.ml.param.shared.HasAggregationDepth.aggregationDepth")
[error]  * abstract method getAggregationDepth()Int in trait org.apache.spark.ml.param.shared.HasAggregationDepth is inherited by class AFTSurvivalRegressionParams in current version.
[error]    filter with: ProblemFilters.exclude[InheritedNewAbstractMethodProblem]("org.apache.spark.ml.param.shared.HasAggregationDepth.getAggregationDepth")
[error]  * abstract synthetic method org$apache$spark$ml$param$shared$HasAggregationDepth$_setter_$aggregationDepth_=(org.apache.spark.ml.param.IntParam)Unit in trait org.apache.spark.ml.param.shared.HasAggregationDepth is inherited by class AFTSurvivalRegressionParams in current version.
[error]    filter with: ProblemFilters.exclude[InheritedNewAbstractMethodProblem]("org.apache.spark.ml.param.shared.HasAggregationDepth.org$apache$spark$ml$param$shared$HasAggregationDepth$_setter_$aggregationDepth_=")

[JoshRosen]

Yep, this new MiMa inspection was added in lightbend-labs/mima#119:

When a new abstract class/trait with abstract members is added to the hierarchy of an existing class/trait, a binary incompatibility must be reported if the abstract members added by the new type did not exist in the hierarchy of the existing class/trait.

Looking at our particular error, it appears that this was introduced by 61ef74f, which added a new HasAggregationDepth trait and then mixed it into other traits.

It looks like HasAggregationDepth actually defines final implementations for all of its methods:

/**
 * Trait for shared param aggregationDepth (default: 2).
 */
private[ml] trait HasAggregationDepth extends Params {

  /**
   * Param for suggested depth for treeAggregate (>= 2).
   * @group param
   */
  final val aggregationDepth: IntParam = new IntParam(this, "aggregationDepth", "suggested depth for treeAggregate (>= 2)", ParamValidators.gtEq(2))

  setDefault(aggregationDepth, 2)

  /** @group getParam */
  final def getAggregationDepth: Int = $(aggregationDepth)
}

I think that a trait with fields / implementations compiles down to both an interface and an abstract class in Scala 2.10 / 2.11, so I imagine that this isn't actually a safe evolution of this class from a Java perspective, even though it technically adds no new abstract members. This is confirmed by one of the test cases in MiMa: https://github.com/dotta/migration-manager/tree/54d54a8d1d7f0da8909c860e734bc1335ddae4c4/reporter/functional-tests/src/test/trait-extending-new-trait-with-concrete-method-nok

@dotta, to clarify the wording in your commit to MiMa, is it accurate to replace "abstract members" by just "members" and say that adding any members to a trait, abstract or concrete, can introduce a binary incompatibility if it wasn't already present in subclasses's existing hierarchy?

[jkbradley]

@JoshRosen I could imagine us introducing this same issue in the future with more Params in sharedParams.scala (like HasAggregationDepth). As long as it's fine for us add MimaExcludes (and are sure Java users won't experience issues), then this seems fine.

[JoshRosen]

I'm not super familiar with MLlib, but are the existing Params subclasses intended to be subclassed by code which lives outside of Spark? If not, then I think we should be okay in terms of compatibility.

[jkbradley]

They currently aren't, but a lot of users want them to be. To make them available outside of Spark, we could create corresponding Java interfaces for them and expose those interfaces instead of the Scala traits.

[JoshRosen]

Gotcha. For now, I've added MiMa excludes to fix the build and plan to merge this as soon as tests pass since I want to fix the flakiness in our builds. We can revisit removing the exclude and fixing this later if we want to.

[SparkQA]

Test build #67307 has finished for PR 15571 at commit c83faa5.

• This patch passes all tests.
• This patch merges cleanly.
• This patch adds no public classes.

[JoshRosen]

I've also merged this into branch-2.0 (2.0.2)