[SPARK-39446][MLLIB][FOLLOWUP] Modify constructor of RankingMetrics class

[uchiiii]

What changes were proposed in this pull request?

• Merged the two constructor into one using RDD[_ <: Product].

Why are the changes needed?

• To make code simpler.
• To support even more inputs.
• The previous code treats rel as an empty array when rel is not provided, which is not that beautiful. This change removes this.

Does this PR introduce any user-facing change?

NO

How was this patch tested?

[zhengruifeng]

I think we may create a private rdd, and than use it internally to minimize changes.

something like this:

private val rdd = predictionAndLabels.map {
    case (pred: Array[T], lab: Array[T]) => ...
    case (pred: Array[T], lab: Array[T], rel: Array[Double]) => ...
}

[uchiiii]

Do you mean that we use rdd as a private variable whose type is RDD[(Array[T], Array[T], Array[Double])], and keep other methods almost the same?

[zhengruifeng]

yes, hope this can reduce modifications

[AmplabJenkins]

Can one of the admins verify this patch?

[uchiiii]

Which do you think is better for ndcgAt?

• The previous one, where to use binary is decided based on whether rel is an empty array.
• The current one, where to use binary is decided based on user input directly.

IMHO, the current one is easier to understand.

[uchiiii]

@zhengruifeng　@srowen
Sorry to interrupt you, but which do you think is better?

[zhengruifeng]

I think The previous one maybe more concise.

[uchiiii]

Thank you for your opinion.

Hm, I thought the current one may be more concise for the developers because you could easily understand the calculation process is different by input type (even though I wrote both of them).

Anyway, I changed this to the previous one.

[srowen]

Maybe we can rename this and the constructor arg; the constructor arg may also have relevance; this does not

[uchiiii]

We may want not to change the name because

• MulticlassMetrics also has the arguments whose name is predictionAndLabels.
  

  spark/mllib/src/main/scala/org/apache/spark/mllib/evaluation/MulticlassMetrics.scala

  Lines 28 to 35 in b588d07

  	/**
  	* Evaluator for multiclass classification.
  	*
  	* @param predictionAndLabels an RDD of (prediction, label, weight, probability) or
  	* (prediction, label, weight) or (prediction, label) tuples.
  	*/
  	@Since("1.1.0")
  	class MulticlassMetrics @Since("1.1.0") (predictionAndLabels: RDD[_ <: Product]) {

• We change the interface of the constructor to be able to support more inputs. Specific names like predictionAndLabelsWithOptionalRelevance may go against the goal.

[srowen]

OK that's reasonable

[zhengruifeng]

why discarding rel here?

If rdd is a RDD[Array[T], Array[T], Array[Double]], then in the ndcgAt, we can simply check whether rel is empty?

also, maybe we can rename rdd to a more meaningful name

[srowen]

WDYT @uchiiii ?

[uchiiii]

How about fullRDD?
IMO, Names like predictionsAndLabelsAndRelevances are too long.

[zhengruifeng]

maybe predictionsLabelsRelevances?

[srowen]

Merged to master