[SW-2646] Calculate Metrics on Arbitrary Dataset #2745
Conversation
mn-mikke
force-pushed
the
mn/SW-2646b
branch
3 times, most recently
from
89d3df6
to
e679aea
Compare
ml/src/test/scala/ai/h2o/sparkling/ml/metrics/BinomialMetricsTestSuite.scala
Outdated
Show resolved
Hide resolved
|
|
||
| override protected def getPredictionValues(dataType: DataType, domain: Array[String], row: Row): Array[Double] = { | ||
| dataType match { | ||
| case StructType(fields) |
There was a problem hiding this comment.
nit: matching like StructType(Array(firstField, secondField)) would also ensure the field counts and avoid calling by index later on
There was a problem hiding this comment.
IMHO, the original approach is more readable. The pattern for the suggested approach would be:
StructType(Array(StructField(_, StringType, _, _), StructField(_, StructType(Array(StructField(_, DoubleType, _, _), StructField(_, DoubleType, _, _))), _, _)))
There was a problem hiding this comment.
The following "long" predicate appears several times in code. Would it be possible to define an auxiliary function that describes the condition and use it, e.g.
def isSomeSpecialType(fields: Array[StructField]): Boolean = fields(0).dataType == StringType && fields(1).dataType.isInstanceOf[StructType] && fields(1).dataType.asInstanceOf[StructType].fields.forall(_.dataType == DoubleType) && fields(1).dataType.asInstanceOf[StructType].fields.length == 2
scoring/src/main/scala/ai/h2o/sparkling/ml/metrics/H2OBinomialMetrics.scala
Outdated
Show resolved
Hide resolved
scoring/src/main/scala/ai/h2o/sparkling/ml/metrics/H2OBinomialMetrics.scala
Outdated
Show resolved
Hide resolved
scoring/src/main/scala/ai/h2o/sparkling/ml/metrics/MetricCalculation.scala
Outdated
Show resolved
Hide resolved
|
LGTM, great piece of work 👍 |
|
@krasinski thanks for your review, much appreciated! I've tried address the vast majority of your comments. |
|
|
||
| override protected def getPredictionValues(dataType: DataType, domain: Array[String], row: Row): Array[Double] = { | ||
| dataType match { | ||
| case StructType(fields) |
There was a problem hiding this comment.
The following "long" predicate appears several times in code. Would it be possible to define an auxiliary function that describes the condition and use it, e.g.
def isSomeSpecialType(fields: Array[StructField]): Boolean = fields(0).dataType == StringType && fields(1).dataType.isInstanceOf[StructType] && fields(1).dataType.asInstanceOf[StructType].fields.forall(_.dataType == DoubleType) && fields(1).dataType.asInstanceOf[StructType].fields.length == 2
No description provided.