You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by mengxr <gi...@git.apache.org> on 2015/03/06 04:46:55 UTC

[GitHub] spark pull request: [SPARK-4588] ML Attributes

GitHub user mengxr opened a pull request:

    https://github.com/apache/spark/pull/4925

    [SPARK-4588] ML Attributes

    This continues the work in #4460 from @srowen . The design doc is published on the JIRA page with some minor changes.

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/mengxr/spark SPARK-4588-new

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/spark/pull/4925.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #4925
    
----
commit 2a21d6d4ab294c515ec23980cd9eda4508576a05
Author: Sean Owen <so...@cloudera.com>
Date:   2015-02-08T22:11:22Z

    Initial draft of FeatureAttributes class

commit 7c944da8b2a1aa9ec9d70fabe77b17d54c08b291
Author: Sean Owen <so...@cloudera.com>
Date:   2015-02-21T12:54:31Z

    Add FeatureType hierarchy and categorical cardinality

commit e7ab467707fbea5a3a7481b863712653a02a6f8d
Author: Xiangrui Meng <me...@databricks.com>
Date:   2015-03-06T03:12:56Z

    update ML attribute impl

commit b1aceefaf9738e7528c179b43bfc99c793160164
Author: Xiangrui Meng <me...@databricks.com>
Date:   2015-03-06T03:44:13Z

    more tests

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-4588] ML Attributes

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/4925#issuecomment-77507469
  
      [Test build #28326 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/28326/consoleFull) for   PR 4925 at commit [`b1aceef`](https://github.com/apache/spark/commit/b1aceefaf9738e7528c179b43bfc99c793160164).
     * This patch **passes all tests**.
     * This patch merges cleanly.
     * This patch adds the following public classes _(experimental)_:
      * `sealed abstract class AttributeType(val name: String)`
      * `sealed abstract class Attribute extends Serializable `



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-4588] ML Attributes

Posted by mengxr <gi...@git.apache.org>.
Github user mengxr commented on a diff in the pull request:

    https://github.com/apache/spark/pull/4925#discussion_r25985352
  
    --- Diff: mllib/src/main/scala/org/apache/spark/ml/attribute/attributes.scala ---
    @@ -0,0 +1,512 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *    http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.spark.ml.attribute
    +
    +import scala.annotation.varargs
    +
    +import org.apache.spark.sql.types.{DoubleType, Metadata, MetadataBuilder, StructField}
    +
    +/**
    + * Abstract class for ML attributes.
    + */
    +sealed abstract class Attribute extends Serializable {
    +
    +  name.foreach { n =>
    +    require(n.nonEmpty, "Cannot have an empty string for name.")
    +  }
    +  index.foreach { i =>
    +    require(i >= 0, s"Index cannot be negative but got $i")
    +  }
    +
    +  /** Attribute type. */
    +  def attrType: AttributeType
    +
    +  /** Name of the attribute. None if it is not set. */
    +  def name: Option[String]
    +
    +  /** Copy with a new name. */
    +  def withName(name: String): Attribute
    +
    +  /** Copy without the name. */
    +  def withoutName: Attribute
    +
    +  /** Index of the attribute. None if it is not set. */
    +  def index: Option[Int]
    +
    +  /** Copy with a new index. */
    +  def withIndex(index: Int): Attribute
    +
    +  /** Copy without the index. */
    +  def withoutIndex: Attribute
    +
    +  /**
    +   * Tests whether this attribute is numeric, true for [[NumericAttribute]] and [[BinaryAttribute]].
    +   */
    +  def isNumeric: Boolean
    +
    +  /**
    +   * Tests whether this attribute is nominal, true for [[NominalAttribute]] and [[BinaryAttribute]].
    +   */
    +  def isNominal: Boolean
    +
    +  /**
    +   * Converts this attribute to [[Metadata]].
    +   * @param withType whether to include the type info
    +   */
    +  private[attribute] def toMetadata(withType: Boolean): Metadata
    +
    +  /**
    +   * Converts this attribute to [[Metadata]]. For numeric attributes, the type info is excluded to
    +   * save space, because numeric type is the default attribute type. For nominal and binary
    +   * attributes, the type info is included.
    +   */
    +  private[attribute] def toMetadata(): Metadata = {
    +    if (attrType == AttributeType.Numeric) {
    +      toMetadata(withType = false)
    +    } else {
    +      toMetadata(withType = true)
    +    }
    +  }
    +
    +  /**
    +   * Converts to a [[StructField]] with some existing metadata.
    +   * @param existingMetadata existing metadata to carry over
    +   */
    +  def toStructField(existingMetadata: Metadata): StructField = {
    +    val newMetadata = new MetadataBuilder()
    +      .withMetadata(existingMetadata)
    +      .putMetadata(AttributeKeys.ML_ATTR, withoutName.withoutIndex.toMetadata())
    +      .build()
    +    StructField(name.get, DoubleType, nullable = false, newMetadata)
    +  }
    +
    +  /** Converts to a [[StructField]]. */
    +  def toStructField(): StructField = toStructField(Metadata.empty)
    +
    +  override def toString: String = toMetadata(withType = true).toString
    +}
    +
    +/** Trait for ML attribute factories. */
    +private[attribute] trait AttributeFactory {
    +
    +  /**
    +   * Creates an [[Attribute]] from a [[Metadata]] instance.
    +   */
    +  private[attribute] def fromMetadata(metadata: Metadata): Attribute
    +
    +  /**
    +   * Creates an [[Attribute]] from a [[StructField]] instance.
    +   */
    +  def fromStructField(field: StructField): Attribute = {
    +    require(field.dataType == DoubleType)
    +    fromMetadata(field.metadata.getMetadata(AttributeKeys.ML_ATTR)).withName(field.name)
    +  }
    +}
    +
    +object Attribute extends AttributeFactory {
    +
    +  private[attribute] override def fromMetadata(metadata: Metadata): Attribute = {
    +    import org.apache.spark.ml.attribute.AttributeKeys._
    +    val attrType = if (metadata.contains(TYPE)) {
    +      metadata.getString(TYPE)
    +    } else {
    +      AttributeType.Numeric.name
    +    }
    +    getFactory(attrType).fromMetadata(metadata)
    +  }
    +
    +  /** Gets the attribute factory given the attribute type name. */
    +  private def getFactory(attrType: String): AttributeFactory = {
    +    if (attrType == AttributeType.Numeric.name) {
    +      NumericAttribute
    +    } else if (attrType == AttributeType.Nominal.name) {
    +      NominalAttribute
    +    } else if (attrType == AttributeType.Binary.name) {
    +      BinaryAttribute
    +    } else {
    +      throw new IllegalArgumentException(s"Cannot recognize type $attrType.")
    +    }
    +  }
    +}
    +
    +
    +/**
    + * A numeric attribute with optional summary statistics.
    + * @param name optional name
    + * @param index optional index
    + * @param min optional min value
    + * @param max optional max value
    + * @param std optional standard deviation
    + * @param sparsity optional sparsity (ratio of zeros)
    + */
    +class NumericAttribute private[ml] (
    +    override val name: Option[String] = None,
    +    override val index: Option[Int] = None,
    +    val min: Option[Double] = None,
    +    val max: Option[Double] = None,
    +    val std: Option[Double] = None,
    +    val sparsity: Option[Double] = None) extends Attribute {
    +
    +  std.foreach { s =>
    +    require(s >= 0.0, s"Standard deviation cannot be negative but got $s.")
    +  }
    +  sparsity.foreach { s =>
    +    require(s >= 0.0 && s <= 1.0, s"Sparsity must be in [0, 1] but got $s.")
    +  }
    +
    +  override def attrType: AttributeType = AttributeType.Numeric
    +
    +  override def withName(name: String): NumericAttribute = copy(name = Some(name))
    +  override def withoutName: NumericAttribute = copy(name = None)
    +
    +  override def withIndex(index: Int): NumericAttribute = copy(index = Some(index))
    +  override def withoutIndex: NumericAttribute = copy(index = None)
    +
    +  /** Copy with a new min value. */
    +  def withMin(min: Double): NumericAttribute = copy(min = Some(min))
    +
    +  /** Copy without the min value. */
    +  def withoutMin: NumericAttribute = copy(min = None)
    +
    +
    +  /** Copy with a new max value. */
    +  def withMax(max: Double): NumericAttribute = copy(max = Some(max))
    +
    +  /** Copy without the max value. */
    +  def withoutMax: NumericAttribute = copy(max = None)
    +
    +  /** Copy with a new standard deviation. */
    +  def withStd(std: Double): NumericAttribute = copy(std = Some(std))
    +
    +  /** Copy without the standard deviation. */
    +  def withoutStd: NumericAttribute = copy(std = None)
    +
    +  /** Copy with a new sparsity. */
    +  def withSparsity(sparsity: Double): NumericAttribute = copy(sparsity = Some(sparsity))
    +
    +  /** Copy without the sparsity. */
    +  def withoutSparsity: NumericAttribute = copy(sparsity = None)
    +
    +  /** Copy without summary statistics. */
    +  def withoutSummary: NumericAttribute = copy(min = None, max = None, std = None, sparsity = None)
    +
    +  override def isNumeric: Boolean = true
    +
    +  override def isNominal: Boolean = false
    +
    +  /** Convert this attribute to metadata. */
    +  private[attribute] override def toMetadata(withType: Boolean): Metadata = {
    +    import org.apache.spark.ml.attribute.AttributeKeys._
    +    val bldr = new MetadataBuilder()
    +    if (withType) bldr.putString(TYPE, attrType.name)
    +    name.foreach(bldr.putString(NAME, _))
    +    index.foreach(bldr.putLong(INDEX, _))
    +    min.foreach(bldr.putDouble(MIN, _))
    +    max.foreach(bldr.putDouble(MAX, _))
    +    std.foreach(bldr.putDouble(STD, _))
    +    sparsity.foreach(bldr.putDouble(SPARSITY, _))
    +    bldr.build()
    +  }
    +
    +  /** Creates a copy of this attribute with optional changes. */
    +  private def copy(
    +      name: Option[String] = name,
    +      index: Option[Int] = index,
    +      min: Option[Double] = min,
    +      max: Option[Double] = max,
    +      std: Option[Double] = std,
    +      sparsity: Option[Double] = sparsity): NumericAttribute = {
    +    new NumericAttribute(name, index, min, max, std, sparsity)
    +  }
    +
    +  override def equals(other: Any): Boolean = {
    +    other match {
    +      case o: NumericAttribute =>
    +        (name == o.name) &&
    +          (index == o.index) &&
    +          (min == o.min) &&
    +          (max == o.max) &&
    +          (std == o.std) &&
    +          (sparsity == o.sparsity)
    +      case _ =>
    +        false
    +    }
    +  }
    +
    +  override def hashCode: Int = {
    +    var sum = 17
    +    sum = 37 * sum + name.hashCode
    +    sum = 37 * sum + index.hashCode
    +    sum = 37 * sum + min.hashCode
    +    sum = 37 * sum + max.hashCode
    +    sum = 37 * sum + std.hashCode
    +    sum = 37 * sum + sparsity.hashCode
    +    sum
    +  }
    +}
    +
    +/**
    + * Factory methods for numeric attributes.
    + */
    +object NumericAttribute extends AttributeFactory {
    +
    +  /** The default numeric attribute. */
    +  val defaultAttr: NumericAttribute = new NumericAttribute
    +
    +  private[attribute] override def fromMetadata(metadata: Metadata): NumericAttribute = {
    +    import org.apache.spark.ml.attribute.AttributeKeys._
    +    val name = if (metadata.contains(NAME)) Some(metadata.getString(NAME)) else None
    +    val index = if (metadata.contains(INDEX)) Some(metadata.getLong(INDEX).toInt) else None
    +    val min = if (metadata.contains(MIN)) Some(metadata.getDouble(MIN)) else None
    +    val max = if (metadata.contains(MAX)) Some(metadata.getDouble(MAX)) else None
    +    val std = if (metadata.contains(STD)) Some(metadata.getDouble(STD)) else None
    +    val sparsity = if (metadata.contains(SPARSITY)) Some(metadata.getDouble(SPARSITY)) else None
    +    new NumericAttribute(name, index, min, max, std, sparsity)
    +  }
    +}
    +
    +/**
    + * A nominal attribute.
    + * @param name optional name
    + * @param index optional index
    + * @param isOrdinal whether this attribute is ordinal (optional)
    + * @param numValues optional number of values. At most one of `numValues` and `values` can be
    + *                  defined.
    + * @param values optional values. At most one of `numValues` and `values` can be defined.
    + */
    +class NominalAttribute private[ml] (
    +    override val name: Option[String] = None,
    +    override val index: Option[Int] = None,
    +    val isOrdinal: Option[Boolean] = None,
    +    val numValues: Option[Int] = None,
    +    val values: Option[Array[String]] = None) extends Attribute {
    +
    +  numValues.foreach { n =>
    +    require(n >= 0, s"numValues cannot be negative but got $n.")
    +  }
    +  require(!(numValues.isDefined && values.isDefined),
    +    "Cannot have both numValues and values defined.")
    +
    +  override def attrType: AttributeType = AttributeType.Nominal
    +
    +  override def isNumeric: Boolean = false
    +
    +  override def isNominal: Boolean = true
    +
    +  private lazy val valueToIndex: Map[String, Int] = {
    +    values.map(_.zipWithIndex.toMap).getOrElse(Map.empty)
    +  }
    +
    +  /** Index of a specific value. */
    +  def indexOf(value: String): Int = {
    +    valueToIndex(value)
    +  }
    +
    +  /** Tests whether this attribute contains a specific value. */
    +  def hasValue(value: String): Boolean = valueToIndex.contains(value)
    +
    +  /** Gets a value given its index. */
    +  def getValue(index: Int): String = values.get(index)
    +
    +  override def withName(name: String): NominalAttribute = copy(name = Some(name))
    +  override def withoutName: NominalAttribute = copy(name = None)
    +
    +  override def withIndex(index: Int): NominalAttribute = copy(index = Some(index))
    +  override def withoutIndex: NominalAttribute = copy(index = None)
    +
    +  /** Copy with new values and empty `numValues`. */
    +  def withValues(values: Array[String]): NominalAttribute = {
    +    copy(numValues = None, values = Some(values))
    +  }
    +
    +  /** Copy with new values and empty `numValues`. */
    +  @varargs
    +  def withValues(first: String, others: String*): NominalAttribute = {
    +    copy(numValues = None, values = Some((first +: others).toArray))
    +  }
    +
    +  /** Copy without the values. */
    +  def withoutValues: NominalAttribute = {
    +    copy(values = None)
    +  }
    +
    +  /** Copy with a new `numValues` and empty `values`. */
    +  def withNumValues(numValues: Int): NominalAttribute = {
    +    copy(numValues = Some(numValues), values = None)
    +  }
    +
    +  /** Copy without the `numValues`. */
    +  def withoutNumValues: NominalAttribute = copy(numValues = None)
    +
    +  /** Creates a copy of this attribute with optional changes. */
    +  private def copy(
    +      name: Option[String] = name,
    +      index: Option[Int] = index,
    +      isOrdinal: Option[Boolean] = isOrdinal,
    +      numValues: Option[Int] = numValues,
    +      values: Option[Array[String]] = values): NominalAttribute = {
    +    new NominalAttribute(name, index, isOrdinal, numValues, values)
    +  }
    +
    +  private[attribute] override def toMetadata(withType: Boolean): Metadata = {
    +    import org.apache.spark.ml.attribute.AttributeKeys._
    +    val bldr = new MetadataBuilder()
    +    if (withType) bldr.putString(TYPE, attrType.name)
    +    name.foreach(bldr.putString(NAME, _))
    +    index.foreach(bldr.putLong(INDEX, _))
    +    isOrdinal.foreach(bldr.putBoolean(ORDINAL, _))
    +    numValues.foreach(bldr.putLong(CARDINALITY, _))
    +    values.foreach(v => bldr.putStringArray(VALUES, v))
    +    bldr.build()
    +  }
    +
    +  override def equals(other: Any): Boolean = {
    +    other match {
    +      case o: NominalAttribute =>
    +        (name == o.name) &&
    +          (index == o.index) &&
    +          (isOrdinal == o.isOrdinal) &&
    +          (numValues == o.numValues) &&
    +          (values.map(_.toSeq) == o.values.map(_.toSeq))
    +      case _ =>
    +        false
    +    }
    +  }
    +
    +  override def hashCode: Int = {
    +    var sum = 17
    +    sum = 37 * sum + name.hashCode
    +    sum = 37 * sum + index.hashCode
    +    sum = 37 * sum + isOrdinal.hashCode
    +    sum = 37 * sum + numValues.hashCode
    +    sum = 37 * sum + values.map(_.toSeq).hashCode
    +    sum
    +  }
    +}
    +
    +/** Factory methods for nominal attributes. */
    +object NominalAttribute extends AttributeFactory {
    +
    +  /** The default nominal attribute. */
    +  final val defaultAttr: NominalAttribute = new NominalAttribute
    +
    +  private[attribute] override def fromMetadata(metadata: Metadata): NominalAttribute = {
    +    import org.apache.spark.ml.attribute.AttributeKeys._
    +    val name = if (metadata.contains(NAME)) Some(metadata.getString(NAME)) else None
    +    val index = if (metadata.contains(INDEX)) Some(metadata.getLong(INDEX).toInt) else None
    +    val isOrdinal = if (metadata.contains(ORDINAL)) Some(metadata.getBoolean(ORDINAL)) else None
    +    val cardinality =
    +      if (metadata.contains(CARDINALITY)) Some(metadata.getLong(CARDINALITY).toInt) else None
    +    val values =
    +      if (metadata.contains(VALUES)) Some(metadata.getStringArray(VALUES)) else None
    +    new NominalAttribute(name, index, isOrdinal, cardinality, values)
    +  }
    +}
    +
    +/**
    + * A binary attribute.
    + * @param name optional name
    + * @param index optional index
    + * @param values optionla values. If set, its size must be 2.
    + */
    +class BinaryAttribute private[ml] (
    --- End diff --
    
    We can define traits like `NumericLike`, `HasSummary`, `NominalLike`. Then `NumericAttribute` extends `NumericLike` with `HasSummary`, `NominalAttribute` extends `NominalLike`, and `BinaryAttribute` extends `NumericLike` and `NominalLike`.
    
    I don't know whether this is really useful in implementing feature transformers. I think it might be better if we merge this, then try a few feature transformers like `LabelIndexer` and `OneHotEncoder` and see whether the APIs are sufficient. Does it sound good to you?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-4588] ML Attributes

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/4925#issuecomment-77611202
  
      [Test build #28349 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/28349/consoleFull) for   PR 4925 at commit [`393ffdc`](https://github.com/apache/spark/commit/393ffdca897b27e4bb263bb538395a045dbedf26).
     * This patch **passes all tests**.
     * This patch merges cleanly.
     * This patch adds the following public classes _(experimental)_:
      * `sealed abstract class AttributeType(val name: String)`
      * `sealed abstract class Attribute extends Serializable `



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-4588] ML Attributes

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/4925#issuecomment-78442381
  
      [Test build #28505 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/28505/consoleFull) for   PR 4925 at commit [`71d1bd0`](https://github.com/apache/spark/commit/71d1bd042360376b7f8f75ba6be3471f443ce34e).
     * This patch merges cleanly.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-4588] ML Attributes

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/spark/pull/4925


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-4588] ML Attributes

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/4925#issuecomment-77651210
  
      [Test build #28357 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/28357/consoleFull) for   PR 4925 at commit [`617be40`](https://github.com/apache/spark/commit/617be409e8a973a93c3ed6d9dcc1b345708e22e4).
     * This patch merges cleanly.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-4588] ML Attributes

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/4925#issuecomment-77659654
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/28357/
    Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-4588] ML Attributes

Posted by mengxr <gi...@git.apache.org>.
Github user mengxr commented on a diff in the pull request:

    https://github.com/apache/spark/pull/4925#discussion_r25988900
  
    --- Diff: mllib/src/main/scala/org/apache/spark/ml/attribute/AttributeGroup.scala ---
    @@ -0,0 +1,234 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *    http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.spark.ml.attribute
    +
    +import scala.collection.mutable.ArrayBuffer
    +
    +import org.apache.spark.mllib.linalg.VectorUDT
    +import org.apache.spark.sql.types.{Metadata, MetadataBuilder, StructField}
    +
    +/**
    + * Attributes that describe a vector ML column.
    + *
    + * @param name name of the attribute group (the ML column name)
    + * @param numAttributes optional number of attributes. At most one of `numAttributes` and `attrs`
    + *                      can be defined.
    + * @param attrs optional array of attributes. Attribute will be copied with their corresponding
    + *              indices in the array.
    + */
    +class AttributeGroup private (
    --- End diff --
    
    Yes, that is correct. We will use both the group name and the attribute name to name to flattened attribute. For example, if `user` is a vector input column, in the flattened column, you see feature names like `user:age` and `user:gender`.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-4588] ML Attributes

Posted by mengxr <gi...@git.apache.org>.
Github user mengxr commented on the pull request:

    https://github.com/apache/spark/pull/4925#issuecomment-78694181
  
    I'm merging this into master, so we can test the APIs in feature transformers. Feel free to propose any changes. Thanks!


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-4588] ML Attributes

Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:

    https://github.com/apache/spark/pull/4925#discussion_r25939339
  
    --- Diff: mllib/src/main/scala/org/apache/spark/ml/attribute/AttributeType.scala ---
    @@ -0,0 +1,61 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *    http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.spark.ml.attribute
    +
    +/**
    + * An enum-like type for attribute types: [[AttributeType$#Numeric]], [[AttributeType$#Nominal]],
    + * and [[AttributeType$#Binary]].
    + */
    +sealed abstract class AttributeType(val name: String)
    +
    +object AttributeType {
    +
    +  /** Numeric type. */
    +  final val Numeric: AttributeType = {
    +    case object Numeric extends AttributeType("numeric")
    +    Numeric
    +  }
    +
    +  /** Nominal type. */
    +  final val Nominal: AttributeType = {
    +    case object Nominal extends AttributeType("nominal")
    +    Nominal
    +  }
    +
    +  /** Binary type. */
    +  final val Binary: AttributeType = {
    +    case object Binary extends AttributeType("binary")
    +    Binary
    +  }
    +
    +  /**
    +   * Gets the [[AttributeType]] object from its name.
    +   * @param name attribute type name: "numeric", "nominal", or "binary"
    +   */
    +  def fromName(name: String): AttributeType = {
    +    if (name == Numeric.name) {
    --- End diff --
    
    Can this be done with `match` or is there a specific advantage to this formulation, like speed?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-4588] ML Attributes

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/4925#issuecomment-77611210
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/28349/
    Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-4588] ML Attributes

Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:

    https://github.com/apache/spark/pull/4925#discussion_r25958956
  
    --- Diff: mllib/src/main/scala/org/apache/spark/ml/attribute/attributes.scala ---
    @@ -0,0 +1,512 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *    http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.spark.ml.attribute
    +
    +import scala.annotation.varargs
    +
    +import org.apache.spark.sql.types.{DoubleType, Metadata, MetadataBuilder, StructField}
    +
    +/**
    + * Abstract class for ML attributes.
    + */
    +sealed abstract class Attribute extends Serializable {
    +
    +  name.foreach { n =>
    +    require(n.nonEmpty, "Cannot have an empty string for name.")
    +  }
    +  index.foreach { i =>
    +    require(i >= 0, s"Index cannot be negative but got $i")
    +  }
    +
    +  /** Attribute type. */
    +  def attrType: AttributeType
    +
    +  /** Name of the attribute. None if it is not set. */
    +  def name: Option[String]
    +
    +  /** Copy with a new name. */
    +  def withName(name: String): Attribute
    +
    +  /** Copy without the name. */
    +  def withoutName: Attribute
    +
    +  /** Index of the attribute. None if it is not set. */
    +  def index: Option[Int]
    +
    +  /** Copy with a new index. */
    +  def withIndex(index: Int): Attribute
    +
    +  /** Copy without the index. */
    +  def withoutIndex: Attribute
    +
    +  /**
    +   * Tests whether this attribute is numeric, true for [[NumericAttribute]] and [[BinaryAttribute]].
    +   */
    +  def isNumeric: Boolean
    +
    +  /**
    +   * Tests whether this attribute is nominal, true for [[NominalAttribute]] and [[BinaryAttribute]].
    +   */
    +  def isNominal: Boolean
    +
    +  /**
    +   * Converts this attribute to [[Metadata]].
    +   * @param withType whether to include the type info
    +   */
    +  private[attribute] def toMetadata(withType: Boolean): Metadata
    +
    +  /**
    +   * Converts this attribute to [[Metadata]]. For numeric attributes, the type info is excluded to
    +   * save space, because numeric type is the default attribute type. For nominal and binary
    +   * attributes, the type info is included.
    +   */
    +  private[attribute] def toMetadata(): Metadata = {
    +    if (attrType == AttributeType.Numeric) {
    +      toMetadata(withType = false)
    +    } else {
    +      toMetadata(withType = true)
    +    }
    +  }
    +
    +  /**
    +   * Converts to a [[StructField]] with some existing metadata.
    +   * @param existingMetadata existing metadata to carry over
    +   */
    +  def toStructField(existingMetadata: Metadata): StructField = {
    +    val newMetadata = new MetadataBuilder()
    +      .withMetadata(existingMetadata)
    +      .putMetadata(AttributeKeys.ML_ATTR, withoutName.withoutIndex.toMetadata())
    +      .build()
    +    StructField(name.get, DoubleType, nullable = false, newMetadata)
    +  }
    +
    +  /** Converts to a [[StructField]]. */
    +  def toStructField(): StructField = toStructField(Metadata.empty)
    +
    +  override def toString: String = toMetadata(withType = true).toString
    +}
    +
    +/** Trait for ML attribute factories. */
    +private[attribute] trait AttributeFactory {
    +
    +  /**
    +   * Creates an [[Attribute]] from a [[Metadata]] instance.
    +   */
    +  private[attribute] def fromMetadata(metadata: Metadata): Attribute
    +
    +  /**
    +   * Creates an [[Attribute]] from a [[StructField]] instance.
    +   */
    +  def fromStructField(field: StructField): Attribute = {
    +    require(field.dataType == DoubleType)
    +    fromMetadata(field.metadata.getMetadata(AttributeKeys.ML_ATTR)).withName(field.name)
    +  }
    +}
    +
    +object Attribute extends AttributeFactory {
    +
    +  private[attribute] override def fromMetadata(metadata: Metadata): Attribute = {
    +    import org.apache.spark.ml.attribute.AttributeKeys._
    +    val attrType = if (metadata.contains(TYPE)) {
    +      metadata.getString(TYPE)
    +    } else {
    +      AttributeType.Numeric.name
    +    }
    +    getFactory(attrType).fromMetadata(metadata)
    +  }
    +
    +  /** Gets the attribute factory given the attribute type name. */
    +  private def getFactory(attrType: String): AttributeFactory = {
    +    if (attrType == AttributeType.Numeric.name) {
    +      NumericAttribute
    +    } else if (attrType == AttributeType.Nominal.name) {
    +      NominalAttribute
    +    } else if (attrType == AttributeType.Binary.name) {
    +      BinaryAttribute
    +    } else {
    +      throw new IllegalArgumentException(s"Cannot recognize type $attrType.")
    +    }
    +  }
    +}
    +
    +
    +/**
    + * A numeric attribute with optional summary statistics.
    + * @param name optional name
    + * @param index optional index
    + * @param min optional min value
    + * @param max optional max value
    + * @param std optional standard deviation
    + * @param sparsity optional sparsity (ratio of zeros)
    + */
    +class NumericAttribute private[ml] (
    +    override val name: Option[String] = None,
    +    override val index: Option[Int] = None,
    +    val min: Option[Double] = None,
    +    val max: Option[Double] = None,
    +    val std: Option[Double] = None,
    +    val sparsity: Option[Double] = None) extends Attribute {
    +
    +  std.foreach { s =>
    +    require(s >= 0.0, s"Standard deviation cannot be negative but got $s.")
    +  }
    +  sparsity.foreach { s =>
    +    require(s >= 0.0 && s <= 1.0, s"Sparsity must be in [0, 1] but got $s.")
    +  }
    +
    +  override def attrType: AttributeType = AttributeType.Numeric
    +
    +  override def withName(name: String): NumericAttribute = copy(name = Some(name))
    +  override def withoutName: NumericAttribute = copy(name = None)
    +
    +  override def withIndex(index: Int): NumericAttribute = copy(index = Some(index))
    +  override def withoutIndex: NumericAttribute = copy(index = None)
    +
    +  /** Copy with a new min value. */
    +  def withMin(min: Double): NumericAttribute = copy(min = Some(min))
    +
    +  /** Copy without the min value. */
    +  def withoutMin: NumericAttribute = copy(min = None)
    +
    +
    +  /** Copy with a new max value. */
    +  def withMax(max: Double): NumericAttribute = copy(max = Some(max))
    +
    +  /** Copy without the max value. */
    +  def withoutMax: NumericAttribute = copy(max = None)
    +
    +  /** Copy with a new standard deviation. */
    +  def withStd(std: Double): NumericAttribute = copy(std = Some(std))
    +
    +  /** Copy without the standard deviation. */
    +  def withoutStd: NumericAttribute = copy(std = None)
    +
    +  /** Copy with a new sparsity. */
    +  def withSparsity(sparsity: Double): NumericAttribute = copy(sparsity = Some(sparsity))
    +
    +  /** Copy without the sparsity. */
    +  def withoutSparsity: NumericAttribute = copy(sparsity = None)
    +
    +  /** Copy without summary statistics. */
    +  def withoutSummary: NumericAttribute = copy(min = None, max = None, std = None, sparsity = None)
    +
    +  override def isNumeric: Boolean = true
    +
    +  override def isNominal: Boolean = false
    +
    +  /** Convert this attribute to metadata. */
    +  private[attribute] override def toMetadata(withType: Boolean): Metadata = {
    +    import org.apache.spark.ml.attribute.AttributeKeys._
    +    val bldr = new MetadataBuilder()
    +    if (withType) bldr.putString(TYPE, attrType.name)
    +    name.foreach(bldr.putString(NAME, _))
    +    index.foreach(bldr.putLong(INDEX, _))
    +    min.foreach(bldr.putDouble(MIN, _))
    +    max.foreach(bldr.putDouble(MAX, _))
    +    std.foreach(bldr.putDouble(STD, _))
    +    sparsity.foreach(bldr.putDouble(SPARSITY, _))
    +    bldr.build()
    +  }
    +
    +  /** Creates a copy of this attribute with optional changes. */
    +  private def copy(
    +      name: Option[String] = name,
    +      index: Option[Int] = index,
    +      min: Option[Double] = min,
    +      max: Option[Double] = max,
    +      std: Option[Double] = std,
    +      sparsity: Option[Double] = sparsity): NumericAttribute = {
    +    new NumericAttribute(name, index, min, max, std, sparsity)
    +  }
    +
    +  override def equals(other: Any): Boolean = {
    +    other match {
    +      case o: NumericAttribute =>
    +        (name == o.name) &&
    +          (index == o.index) &&
    +          (min == o.min) &&
    +          (max == o.max) &&
    +          (std == o.std) &&
    +          (sparsity == o.sparsity)
    +      case _ =>
    +        false
    +    }
    +  }
    +
    +  override def hashCode: Int = {
    +    var sum = 17
    +    sum = 37 * sum + name.hashCode
    +    sum = 37 * sum + index.hashCode
    +    sum = 37 * sum + min.hashCode
    +    sum = 37 * sum + max.hashCode
    +    sum = 37 * sum + std.hashCode
    +    sum = 37 * sum + sparsity.hashCode
    +    sum
    +  }
    +}
    +
    +/**
    + * Factory methods for numeric attributes.
    + */
    +object NumericAttribute extends AttributeFactory {
    +
    +  /** The default numeric attribute. */
    +  val defaultAttr: NumericAttribute = new NumericAttribute
    +
    +  private[attribute] override def fromMetadata(metadata: Metadata): NumericAttribute = {
    +    import org.apache.spark.ml.attribute.AttributeKeys._
    +    val name = if (metadata.contains(NAME)) Some(metadata.getString(NAME)) else None
    +    val index = if (metadata.contains(INDEX)) Some(metadata.getLong(INDEX).toInt) else None
    +    val min = if (metadata.contains(MIN)) Some(metadata.getDouble(MIN)) else None
    +    val max = if (metadata.contains(MAX)) Some(metadata.getDouble(MAX)) else None
    +    val std = if (metadata.contains(STD)) Some(metadata.getDouble(STD)) else None
    +    val sparsity = if (metadata.contains(SPARSITY)) Some(metadata.getDouble(SPARSITY)) else None
    +    new NumericAttribute(name, index, min, max, std, sparsity)
    +  }
    +}
    +
    +/**
    + * A nominal attribute.
    + * @param name optional name
    + * @param index optional index
    + * @param isOrdinal whether this attribute is ordinal (optional)
    + * @param numValues optional number of values. At most one of `numValues` and `values` can be
    + *                  defined.
    + * @param values optional values. At most one of `numValues` and `values` can be defined.
    + */
    +class NominalAttribute private[ml] (
    +    override val name: Option[String] = None,
    +    override val index: Option[Int] = None,
    +    val isOrdinal: Option[Boolean] = None,
    +    val numValues: Option[Int] = None,
    +    val values: Option[Array[String]] = None) extends Attribute {
    +
    +  numValues.foreach { n =>
    +    require(n >= 0, s"numValues cannot be negative but got $n.")
    +  }
    +  require(!(numValues.isDefined && values.isDefined),
    +    "Cannot have both numValues and values defined.")
    +
    +  override def attrType: AttributeType = AttributeType.Nominal
    +
    +  override def isNumeric: Boolean = false
    +
    +  override def isNominal: Boolean = true
    +
    +  private lazy val valueToIndex: Map[String, Int] = {
    +    values.map(_.zipWithIndex.toMap).getOrElse(Map.empty)
    +  }
    +
    +  /** Index of a specific value. */
    +  def indexOf(value: String): Int = {
    +    valueToIndex(value)
    +  }
    +
    +  /** Tests whether this attribute contains a specific value. */
    +  def hasValue(value: String): Boolean = valueToIndex.contains(value)
    +
    +  /** Gets a value given its index. */
    +  def getValue(index: Int): String = values.get(index)
    +
    +  override def withName(name: String): NominalAttribute = copy(name = Some(name))
    +  override def withoutName: NominalAttribute = copy(name = None)
    +
    +  override def withIndex(index: Int): NominalAttribute = copy(index = Some(index))
    +  override def withoutIndex: NominalAttribute = copy(index = None)
    +
    +  /** Copy with new values and empty `numValues`. */
    +  def withValues(values: Array[String]): NominalAttribute = {
    +    copy(numValues = None, values = Some(values))
    +  }
    +
    +  /** Copy with new values and empty `numValues`. */
    +  @varargs
    +  def withValues(first: String, others: String*): NominalAttribute = {
    +    copy(numValues = None, values = Some((first +: others).toArray))
    +  }
    +
    +  /** Copy without the values. */
    +  def withoutValues: NominalAttribute = {
    +    copy(values = None)
    +  }
    +
    +  /** Copy with a new `numValues` and empty `values`. */
    +  def withNumValues(numValues: Int): NominalAttribute = {
    +    copy(numValues = Some(numValues), values = None)
    +  }
    +
    +  /** Copy without the `numValues`. */
    +  def withoutNumValues: NominalAttribute = copy(numValues = None)
    +
    +  /** Creates a copy of this attribute with optional changes. */
    +  private def copy(
    +      name: Option[String] = name,
    +      index: Option[Int] = index,
    +      isOrdinal: Option[Boolean] = isOrdinal,
    +      numValues: Option[Int] = numValues,
    +      values: Option[Array[String]] = values): NominalAttribute = {
    +    new NominalAttribute(name, index, isOrdinal, numValues, values)
    +  }
    +
    +  private[attribute] override def toMetadata(withType: Boolean): Metadata = {
    +    import org.apache.spark.ml.attribute.AttributeKeys._
    +    val bldr = new MetadataBuilder()
    +    if (withType) bldr.putString(TYPE, attrType.name)
    +    name.foreach(bldr.putString(NAME, _))
    +    index.foreach(bldr.putLong(INDEX, _))
    +    isOrdinal.foreach(bldr.putBoolean(ORDINAL, _))
    +    numValues.foreach(bldr.putLong(CARDINALITY, _))
    +    values.foreach(v => bldr.putStringArray(VALUES, v))
    +    bldr.build()
    +  }
    +
    +  override def equals(other: Any): Boolean = {
    +    other match {
    +      case o: NominalAttribute =>
    +        (name == o.name) &&
    +          (index == o.index) &&
    +          (isOrdinal == o.isOrdinal) &&
    +          (numValues == o.numValues) &&
    +          (values.map(_.toSeq) == o.values.map(_.toSeq))
    +      case _ =>
    +        false
    +    }
    +  }
    +
    +  override def hashCode: Int = {
    +    var sum = 17
    +    sum = 37 * sum + name.hashCode
    +    sum = 37 * sum + index.hashCode
    +    sum = 37 * sum + isOrdinal.hashCode
    +    sum = 37 * sum + numValues.hashCode
    +    sum = 37 * sum + values.map(_.toSeq).hashCode
    +    sum
    +  }
    +}
    +
    +/** Factory methods for nominal attributes. */
    +object NominalAttribute extends AttributeFactory {
    +
    +  /** The default nominal attribute. */
    +  final val defaultAttr: NominalAttribute = new NominalAttribute
    +
    +  private[attribute] override def fromMetadata(metadata: Metadata): NominalAttribute = {
    +    import org.apache.spark.ml.attribute.AttributeKeys._
    +    val name = if (metadata.contains(NAME)) Some(metadata.getString(NAME)) else None
    +    val index = if (metadata.contains(INDEX)) Some(metadata.getLong(INDEX).toInt) else None
    +    val isOrdinal = if (metadata.contains(ORDINAL)) Some(metadata.getBoolean(ORDINAL)) else None
    +    val cardinality =
    +      if (metadata.contains(CARDINALITY)) Some(metadata.getLong(CARDINALITY).toInt) else None
    +    val values =
    +      if (metadata.contains(VALUES)) Some(metadata.getStringArray(VALUES)) else None
    +    new NominalAttribute(name, index, isOrdinal, cardinality, values)
    +  }
    +}
    +
    +/**
    + * A binary attribute.
    + * @param name optional name
    + * @param index optional index
    + * @param values optionla values. If set, its size must be 2.
    + */
    +class BinaryAttribute private[ml] (
    --- End diff --
    
    Yeah that's what I was struggling with by declaring a number of traits instead in my PR. There was no good solution although you could sort of get it to work with nominal/numeric traits, with concrete classes extending them. I didn't like it.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-4588] ML Attributes

Posted by mengxr <gi...@git.apache.org>.
Github user mengxr commented on a diff in the pull request:

    https://github.com/apache/spark/pull/4925#discussion_r25988254
  
    --- Diff: mllib/src/main/scala/org/apache/spark/ml/attribute/AttributeGroup.scala ---
    @@ -0,0 +1,234 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *    http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.spark.ml.attribute
    +
    +import scala.collection.mutable.ArrayBuffer
    +
    +import org.apache.spark.mllib.linalg.VectorUDT
    +import org.apache.spark.sql.types.{Metadata, MetadataBuilder, StructField}
    +
    +/**
    + * Attributes that describe a vector ML column.
    + *
    + * @param name name of the attribute group (the ML column name)
    + * @param numAttributes optional number of attributes. At most one of `numAttributes` and `attrs`
    + *                      can be defined.
    + * @param attrs optional array of attributes. Attribute will be copied with their corresponding
    + *              indices in the array.
    + */
    +class AttributeGroup private (
    --- End diff --
    
    You need a `VectorAssembler` to merge those three columns into a single vector column. The `VectorAssembler` will merge the ML attributes as well. In this way, the algorithms only need to handle vector columns.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-4588] ML Attributes

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/4925#issuecomment-77507477
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/28326/
    Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-4588] ML Attributes

Posted by mengxr <gi...@git.apache.org>.
Github user mengxr commented on a diff in the pull request:

    https://github.com/apache/spark/pull/4925#discussion_r25958319
  
    --- Diff: mllib/src/main/scala/org/apache/spark/ml/attribute/attributes.scala ---
    @@ -0,0 +1,512 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *    http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.spark.ml.attribute
    +
    +import scala.annotation.varargs
    +
    +import org.apache.spark.sql.types.{DoubleType, Metadata, MetadataBuilder, StructField}
    +
    +/**
    + * Abstract class for ML attributes.
    + */
    +sealed abstract class Attribute extends Serializable {
    +
    +  name.foreach { n =>
    +    require(n.nonEmpty, "Cannot have an empty string for name.")
    +  }
    +  index.foreach { i =>
    +    require(i >= 0, s"Index cannot be negative but got $i")
    +  }
    +
    +  /** Attribute type. */
    +  def attrType: AttributeType
    +
    +  /** Name of the attribute. None if it is not set. */
    +  def name: Option[String]
    +
    +  /** Copy with a new name. */
    +  def withName(name: String): Attribute
    +
    +  /** Copy without the name. */
    +  def withoutName: Attribute
    +
    +  /** Index of the attribute. None if it is not set. */
    +  def index: Option[Int]
    +
    +  /** Copy with a new index. */
    +  def withIndex(index: Int): Attribute
    +
    +  /** Copy without the index. */
    +  def withoutIndex: Attribute
    +
    +  /**
    +   * Tests whether this attribute is numeric, true for [[NumericAttribute]] and [[BinaryAttribute]].
    +   */
    +  def isNumeric: Boolean
    +
    +  /**
    +   * Tests whether this attribute is nominal, true for [[NominalAttribute]] and [[BinaryAttribute]].
    +   */
    +  def isNominal: Boolean
    +
    +  /**
    +   * Converts this attribute to [[Metadata]].
    +   * @param withType whether to include the type info
    +   */
    +  private[attribute] def toMetadata(withType: Boolean): Metadata
    +
    +  /**
    +   * Converts this attribute to [[Metadata]]. For numeric attributes, the type info is excluded to
    +   * save space, because numeric type is the default attribute type. For nominal and binary
    +   * attributes, the type info is included.
    +   */
    +  private[attribute] def toMetadata(): Metadata = {
    +    if (attrType == AttributeType.Numeric) {
    +      toMetadata(withType = false)
    +    } else {
    +      toMetadata(withType = true)
    +    }
    +  }
    +
    +  /**
    +   * Converts to a [[StructField]] with some existing metadata.
    +   * @param existingMetadata existing metadata to carry over
    +   */
    +  def toStructField(existingMetadata: Metadata): StructField = {
    +    val newMetadata = new MetadataBuilder()
    +      .withMetadata(existingMetadata)
    +      .putMetadata(AttributeKeys.ML_ATTR, withoutName.withoutIndex.toMetadata())
    +      .build()
    +    StructField(name.get, DoubleType, nullable = false, newMetadata)
    +  }
    +
    +  /** Converts to a [[StructField]]. */
    +  def toStructField(): StructField = toStructField(Metadata.empty)
    +
    +  override def toString: String = toMetadata(withType = true).toString
    +}
    +
    +/** Trait for ML attribute factories. */
    +private[attribute] trait AttributeFactory {
    +
    +  /**
    +   * Creates an [[Attribute]] from a [[Metadata]] instance.
    +   */
    +  private[attribute] def fromMetadata(metadata: Metadata): Attribute
    +
    +  /**
    +   * Creates an [[Attribute]] from a [[StructField]] instance.
    +   */
    +  def fromStructField(field: StructField): Attribute = {
    +    require(field.dataType == DoubleType)
    +    fromMetadata(field.metadata.getMetadata(AttributeKeys.ML_ATTR)).withName(field.name)
    +  }
    +}
    +
    +object Attribute extends AttributeFactory {
    +
    +  private[attribute] override def fromMetadata(metadata: Metadata): Attribute = {
    +    import org.apache.spark.ml.attribute.AttributeKeys._
    +    val attrType = if (metadata.contains(TYPE)) {
    +      metadata.getString(TYPE)
    +    } else {
    +      AttributeType.Numeric.name
    +    }
    +    getFactory(attrType).fromMetadata(metadata)
    +  }
    +
    +  /** Gets the attribute factory given the attribute type name. */
    +  private def getFactory(attrType: String): AttributeFactory = {
    +    if (attrType == AttributeType.Numeric.name) {
    +      NumericAttribute
    +    } else if (attrType == AttributeType.Nominal.name) {
    +      NominalAttribute
    +    } else if (attrType == AttributeType.Binary.name) {
    +      BinaryAttribute
    +    } else {
    +      throw new IllegalArgumentException(s"Cannot recognize type $attrType.")
    +    }
    +  }
    +}
    +
    +
    +/**
    + * A numeric attribute with optional summary statistics.
    + * @param name optional name
    + * @param index optional index
    + * @param min optional min value
    + * @param max optional max value
    + * @param std optional standard deviation
    + * @param sparsity optional sparsity (ratio of zeros)
    + */
    +class NumericAttribute private[ml] (
    +    override val name: Option[String] = None,
    +    override val index: Option[Int] = None,
    +    val min: Option[Double] = None,
    +    val max: Option[Double] = None,
    +    val std: Option[Double] = None,
    +    val sparsity: Option[Double] = None) extends Attribute {
    +
    +  std.foreach { s =>
    +    require(s >= 0.0, s"Standard deviation cannot be negative but got $s.")
    +  }
    +  sparsity.foreach { s =>
    +    require(s >= 0.0 && s <= 1.0, s"Sparsity must be in [0, 1] but got $s.")
    +  }
    +
    +  override def attrType: AttributeType = AttributeType.Numeric
    +
    +  override def withName(name: String): NumericAttribute = copy(name = Some(name))
    +  override def withoutName: NumericAttribute = copy(name = None)
    +
    +  override def withIndex(index: Int): NumericAttribute = copy(index = Some(index))
    +  override def withoutIndex: NumericAttribute = copy(index = None)
    +
    +  /** Copy with a new min value. */
    +  def withMin(min: Double): NumericAttribute = copy(min = Some(min))
    +
    +  /** Copy without the min value. */
    +  def withoutMin: NumericAttribute = copy(min = None)
    +
    +
    +  /** Copy with a new max value. */
    +  def withMax(max: Double): NumericAttribute = copy(max = Some(max))
    +
    +  /** Copy without the max value. */
    +  def withoutMax: NumericAttribute = copy(max = None)
    +
    +  /** Copy with a new standard deviation. */
    +  def withStd(std: Double): NumericAttribute = copy(std = Some(std))
    +
    +  /** Copy without the standard deviation. */
    +  def withoutStd: NumericAttribute = copy(std = None)
    +
    +  /** Copy with a new sparsity. */
    +  def withSparsity(sparsity: Double): NumericAttribute = copy(sparsity = Some(sparsity))
    +
    +  /** Copy without the sparsity. */
    +  def withoutSparsity: NumericAttribute = copy(sparsity = None)
    +
    +  /** Copy without summary statistics. */
    +  def withoutSummary: NumericAttribute = copy(min = None, max = None, std = None, sparsity = None)
    +
    +  override def isNumeric: Boolean = true
    +
    +  override def isNominal: Boolean = false
    +
    +  /** Convert this attribute to metadata. */
    +  private[attribute] override def toMetadata(withType: Boolean): Metadata = {
    +    import org.apache.spark.ml.attribute.AttributeKeys._
    +    val bldr = new MetadataBuilder()
    +    if (withType) bldr.putString(TYPE, attrType.name)
    +    name.foreach(bldr.putString(NAME, _))
    +    index.foreach(bldr.putLong(INDEX, _))
    +    min.foreach(bldr.putDouble(MIN, _))
    +    max.foreach(bldr.putDouble(MAX, _))
    +    std.foreach(bldr.putDouble(STD, _))
    +    sparsity.foreach(bldr.putDouble(SPARSITY, _))
    +    bldr.build()
    +  }
    +
    +  /** Creates a copy of this attribute with optional changes. */
    +  private def copy(
    +      name: Option[String] = name,
    +      index: Option[Int] = index,
    +      min: Option[Double] = min,
    +      max: Option[Double] = max,
    +      std: Option[Double] = std,
    +      sparsity: Option[Double] = sparsity): NumericAttribute = {
    +    new NumericAttribute(name, index, min, max, std, sparsity)
    +  }
    +
    +  override def equals(other: Any): Boolean = {
    +    other match {
    +      case o: NumericAttribute =>
    +        (name == o.name) &&
    +          (index == o.index) &&
    +          (min == o.min) &&
    +          (max == o.max) &&
    +          (std == o.std) &&
    +          (sparsity == o.sparsity)
    +      case _ =>
    +        false
    +    }
    +  }
    +
    +  override def hashCode: Int = {
    +    var sum = 17
    +    sum = 37 * sum + name.hashCode
    +    sum = 37 * sum + index.hashCode
    +    sum = 37 * sum + min.hashCode
    +    sum = 37 * sum + max.hashCode
    +    sum = 37 * sum + std.hashCode
    +    sum = 37 * sum + sparsity.hashCode
    +    sum
    +  }
    +}
    +
    +/**
    + * Factory methods for numeric attributes.
    + */
    +object NumericAttribute extends AttributeFactory {
    +
    +  /** The default numeric attribute. */
    +  val defaultAttr: NumericAttribute = new NumericAttribute
    +
    +  private[attribute] override def fromMetadata(metadata: Metadata): NumericAttribute = {
    +    import org.apache.spark.ml.attribute.AttributeKeys._
    +    val name = if (metadata.contains(NAME)) Some(metadata.getString(NAME)) else None
    +    val index = if (metadata.contains(INDEX)) Some(metadata.getLong(INDEX).toInt) else None
    +    val min = if (metadata.contains(MIN)) Some(metadata.getDouble(MIN)) else None
    +    val max = if (metadata.contains(MAX)) Some(metadata.getDouble(MAX)) else None
    +    val std = if (metadata.contains(STD)) Some(metadata.getDouble(STD)) else None
    +    val sparsity = if (metadata.contains(SPARSITY)) Some(metadata.getDouble(SPARSITY)) else None
    +    new NumericAttribute(name, index, min, max, std, sparsity)
    +  }
    +}
    +
    +/**
    + * A nominal attribute.
    + * @param name optional name
    + * @param index optional index
    + * @param isOrdinal whether this attribute is ordinal (optional)
    + * @param numValues optional number of values. At most one of `numValues` and `values` can be
    + *                  defined.
    + * @param values optional values. At most one of `numValues` and `values` can be defined.
    + */
    +class NominalAttribute private[ml] (
    +    override val name: Option[String] = None,
    +    override val index: Option[Int] = None,
    +    val isOrdinal: Option[Boolean] = None,
    +    val numValues: Option[Int] = None,
    +    val values: Option[Array[String]] = None) extends Attribute {
    +
    +  numValues.foreach { n =>
    +    require(n >= 0, s"numValues cannot be negative but got $n.")
    +  }
    +  require(!(numValues.isDefined && values.isDefined),
    +    "Cannot have both numValues and values defined.")
    +
    +  override def attrType: AttributeType = AttributeType.Nominal
    +
    +  override def isNumeric: Boolean = false
    +
    +  override def isNominal: Boolean = true
    +
    +  private lazy val valueToIndex: Map[String, Int] = {
    +    values.map(_.zipWithIndex.toMap).getOrElse(Map.empty)
    +  }
    +
    +  /** Index of a specific value. */
    +  def indexOf(value: String): Int = {
    +    valueToIndex(value)
    +  }
    +
    +  /** Tests whether this attribute contains a specific value. */
    +  def hasValue(value: String): Boolean = valueToIndex.contains(value)
    +
    +  /** Gets a value given its index. */
    +  def getValue(index: Int): String = values.get(index)
    +
    +  override def withName(name: String): NominalAttribute = copy(name = Some(name))
    +  override def withoutName: NominalAttribute = copy(name = None)
    +
    +  override def withIndex(index: Int): NominalAttribute = copy(index = Some(index))
    +  override def withoutIndex: NominalAttribute = copy(index = None)
    +
    +  /** Copy with new values and empty `numValues`. */
    +  def withValues(values: Array[String]): NominalAttribute = {
    +    copy(numValues = None, values = Some(values))
    +  }
    +
    +  /** Copy with new values and empty `numValues`. */
    +  @varargs
    +  def withValues(first: String, others: String*): NominalAttribute = {
    +    copy(numValues = None, values = Some((first +: others).toArray))
    +  }
    +
    +  /** Copy without the values. */
    +  def withoutValues: NominalAttribute = {
    +    copy(values = None)
    +  }
    +
    +  /** Copy with a new `numValues` and empty `values`. */
    +  def withNumValues(numValues: Int): NominalAttribute = {
    +    copy(numValues = Some(numValues), values = None)
    +  }
    +
    +  /** Copy without the `numValues`. */
    +  def withoutNumValues: NominalAttribute = copy(numValues = None)
    +
    +  /** Creates a copy of this attribute with optional changes. */
    +  private def copy(
    +      name: Option[String] = name,
    +      index: Option[Int] = index,
    +      isOrdinal: Option[Boolean] = isOrdinal,
    +      numValues: Option[Int] = numValues,
    +      values: Option[Array[String]] = values): NominalAttribute = {
    +    new NominalAttribute(name, index, isOrdinal, numValues, values)
    +  }
    +
    +  private[attribute] override def toMetadata(withType: Boolean): Metadata = {
    +    import org.apache.spark.ml.attribute.AttributeKeys._
    +    val bldr = new MetadataBuilder()
    +    if (withType) bldr.putString(TYPE, attrType.name)
    +    name.foreach(bldr.putString(NAME, _))
    +    index.foreach(bldr.putLong(INDEX, _))
    +    isOrdinal.foreach(bldr.putBoolean(ORDINAL, _))
    +    numValues.foreach(bldr.putLong(CARDINALITY, _))
    +    values.foreach(v => bldr.putStringArray(VALUES, v))
    +    bldr.build()
    +  }
    +
    +  override def equals(other: Any): Boolean = {
    +    other match {
    +      case o: NominalAttribute =>
    +        (name == o.name) &&
    +          (index == o.index) &&
    +          (isOrdinal == o.isOrdinal) &&
    +          (numValues == o.numValues) &&
    +          (values.map(_.toSeq) == o.values.map(_.toSeq))
    +      case _ =>
    +        false
    +    }
    +  }
    +
    +  override def hashCode: Int = {
    +    var sum = 17
    +    sum = 37 * sum + name.hashCode
    +    sum = 37 * sum + index.hashCode
    +    sum = 37 * sum + isOrdinal.hashCode
    +    sum = 37 * sum + numValues.hashCode
    +    sum = 37 * sum + values.map(_.toSeq).hashCode
    +    sum
    +  }
    +}
    +
    +/** Factory methods for nominal attributes. */
    +object NominalAttribute extends AttributeFactory {
    +
    +  /** The default nominal attribute. */
    +  final val defaultAttr: NominalAttribute = new NominalAttribute
    +
    +  private[attribute] override def fromMetadata(metadata: Metadata): NominalAttribute = {
    +    import org.apache.spark.ml.attribute.AttributeKeys._
    +    val name = if (metadata.contains(NAME)) Some(metadata.getString(NAME)) else None
    +    val index = if (metadata.contains(INDEX)) Some(metadata.getLong(INDEX).toInt) else None
    +    val isOrdinal = if (metadata.contains(ORDINAL)) Some(metadata.getBoolean(ORDINAL)) else None
    +    val cardinality =
    +      if (metadata.contains(CARDINALITY)) Some(metadata.getLong(CARDINALITY).toInt) else None
    +    val values =
    +      if (metadata.contains(VALUES)) Some(metadata.getStringArray(VALUES)) else None
    +    new NominalAttribute(name, index, isOrdinal, cardinality, values)
    +  }
    +}
    +
    +/**
    + * A binary attribute.
    + * @param name optional name
    + * @param index optional index
    + * @param values optionla values. If set, its size must be 2.
    + */
    +class BinaryAttribute private[ml] (
    --- End diff --
    
    I put `isNominal` and `isNumeric` in the `Attribute` field. We can let it extend `NominalAttribute` but then the question is whether we should let it extend `NumericAttribute` too. But it cannot extend both technically.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-4588] ML Attributes

Posted by mengxr <gi...@git.apache.org>.
Github user mengxr commented on a diff in the pull request:

    https://github.com/apache/spark/pull/4925#discussion_r25962561
  
    --- Diff: mllib/src/main/scala/org/apache/spark/ml/attribute/AttributeKeys.scala ---
    @@ -0,0 +1,37 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *    http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.spark.ml.attribute
    +
    +/**
    + * Keys used to store attributes.
    + */
    +private[attribute] object AttributeKeys {
    +  final val ML_ATTR: String = "ml_attr"
    --- End diff --
    
    For public fields, we put the types explicitly. You are right about `final`. An `object` is already `final`, we don't need `final` for its members. I'lll remove them with the next update.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-4588] ML Attributes

Posted by mengxr <gi...@git.apache.org>.
Github user mengxr commented on a diff in the pull request:

    https://github.com/apache/spark/pull/4925#discussion_r25958650
  
    --- Diff: mllib/src/main/scala/org/apache/spark/ml/attribute/AttributeGroup.scala ---
    @@ -0,0 +1,234 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *    http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.spark.ml.attribute
    +
    +import scala.collection.mutable.ArrayBuffer
    +
    +import org.apache.spark.mllib.linalg.VectorUDT
    +import org.apache.spark.sql.types.{Metadata, MetadataBuilder, StructField}
    +
    +/**
    + * Attributes that describe a vector ML column.
    + *
    + * @param name name of the attribute group (the ML column name)
    + * @param numAttributes optional number of attributes. At most one of `numAttributes` and `attrs`
    + *                      can be defined.
    + * @param attrs optional array of attributes. Attribute will be copied with their corresponding
    + *              indices in the array.
    + */
    +class AttributeGroup private (
    --- End diff --
    
    There are no nested groups. An ML column is either a Double column or a Vector column.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-4588] ML Attributes

Posted by mengxr <gi...@git.apache.org>.
Github user mengxr commented on a diff in the pull request:

    https://github.com/apache/spark/pull/4925#discussion_r25961138
  
    --- Diff: mllib/src/main/scala/org/apache/spark/ml/attribute/AttributeType.scala ---
    @@ -0,0 +1,61 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *    http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.spark.ml.attribute
    +
    +/**
    + * An enum-like type for attribute types: [[AttributeType$#Numeric]], [[AttributeType$#Nominal]],
    + * and [[AttributeType$#Binary]].
    + */
    +sealed abstract class AttributeType(val name: String)
    +
    +object AttributeType {
    +
    +  /** Numeric type. */
    +  final val Numeric: AttributeType = {
    +    case object Numeric extends AttributeType("numeric")
    +    Numeric
    +  }
    +
    +  /** Nominal type. */
    +  final val Nominal: AttributeType = {
    +    case object Nominal extends AttributeType("nominal")
    +    Nominal
    +  }
    +
    +  /** Binary type. */
    +  final val Binary: AttributeType = {
    +    case object Binary extends AttributeType("binary")
    +    Binary
    +  }
    +
    +  /**
    +   * Gets the [[AttributeType]] object from its name.
    +   * @param name attribute type name: "numeric", "nominal", or "binary"
    +   */
    +  def fromName(name: String): AttributeType = {
    +    if (name == Numeric.name) {
    --- End diff --
    
    Verified that `match ... case ...` produces more byte code. It doesn't mean that there will be a noticeable performance drop. No strong preference here.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-4588] ML Attributes

Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:

    https://github.com/apache/spark/pull/4925#discussion_r25988627
  
    --- Diff: mllib/src/main/scala/org/apache/spark/ml/attribute/AttributeGroup.scala ---
    @@ -0,0 +1,234 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *    http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.spark.ml.attribute
    +
    +import scala.collection.mutable.ArrayBuffer
    +
    +import org.apache.spark.mllib.linalg.VectorUDT
    +import org.apache.spark.sql.types.{Metadata, MetadataBuilder, StructField}
    +
    +/**
    + * Attributes that describe a vector ML column.
    + *
    + * @param name name of the attribute group (the ML column name)
    + * @param numAttributes optional number of attributes. At most one of `numAttributes` and `attrs`
    + *                      can be defined.
    + * @param attrs optional array of attributes. Attribute will be copied with their corresponding
    + *              indices in the array.
    + */
    +class AttributeGroup private (
    --- End diff --
    
    Does that effectively 'flatten' the schema, you mean? so if I have two numeric columns, and one vector-valued columns with 3 features inside, then the assembler makes a representation of 5 features? If that's right, then yes now I get what you mean by this. So, there is nothing here that represents the original schema with its nesting, since that would never be how it is presented to an algorithm, right?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-4588] ML Attributes

Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:

    https://github.com/apache/spark/pull/4925#discussion_r25985633
  
    --- Diff: mllib/src/main/scala/org/apache/spark/ml/attribute/AttributeGroup.scala ---
    @@ -0,0 +1,234 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *    http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.spark.ml.attribute
    +
    +import scala.collection.mutable.ArrayBuffer
    +
    +import org.apache.spark.mllib.linalg.VectorUDT
    +import org.apache.spark.sql.types.{Metadata, MetadataBuilder, StructField}
    +
    +/**
    + * Attributes that describe a vector ML column.
    + *
    + * @param name name of the attribute group (the ML column name)
    + * @param numAttributes optional number of attributes. At most one of `numAttributes` and `attrs`
    + *                      can be defined.
    + * @param attrs optional array of attributes. Attribute will be copied with their corresponding
    + *              indices in the array.
    + */
    +class AttributeGroup private (
    --- End diff --
    
    OK if this is the abstraction for a vector-valued column, what is the abstraction for the overall set of features? Let's say I have two numeric columns and one vector-valued column -- where is that represented?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-4588] ML Attributes

Posted by mengxr <gi...@git.apache.org>.
Github user mengxr commented on a diff in the pull request:

    https://github.com/apache/spark/pull/4925#discussion_r25958607
  
    --- Diff: mllib/src/main/scala/org/apache/spark/ml/attribute/AttributeType.scala ---
    @@ -0,0 +1,61 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *    http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.spark.ml.attribute
    +
    +/**
    + * An enum-like type for attribute types: [[AttributeType$#Numeric]], [[AttributeType$#Nominal]],
    + * and [[AttributeType$#Binary]].
    + */
    +sealed abstract class AttributeType(val name: String)
    +
    +object AttributeType {
    +
    +  /** Numeric type. */
    +  final val Numeric: AttributeType = {
    +    case object Numeric extends AttributeType("numeric")
    +    Numeric
    +  }
    +
    +  /** Nominal type. */
    +  final val Nominal: AttributeType = {
    +    case object Nominal extends AttributeType("nominal")
    +    Nominal
    +  }
    +
    +  /** Binary type. */
    +  final val Binary: AttributeType = {
    +    case object Binary extends AttributeType("binary")
    +    Binary
    +  }
    +
    +  /**
    +   * Gets the [[AttributeType]] object from its name.
    +   * @param name attribute type name: "numeric", "nominal", or "binary"
    +   */
    +  def fromName(name: String): AttributeType = {
    +    if (name == Numeric.name) {
    --- End diff --
    
    Yes. `match ... case ...` usually generates more byte code, but I didn't verify it in this case. Let me check the bytecode.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-4588] ML Attributes

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/4925#issuecomment-78452442
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/28505/
    Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-4588] ML Attributes

Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:

    https://github.com/apache/spark/pull/4925#discussion_r25939180
  
    --- Diff: mllib/src/main/scala/org/apache/spark/ml/attribute/AttributeKeys.scala ---
    @@ -0,0 +1,37 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *    http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.spark.ml.attribute
    +
    +/**
    + * Keys used to store attributes.
    + */
    +private[attribute] object AttributeKeys {
    +  final val ML_ATTR: String = "ml_attr"
    --- End diff --
    
    Do you need `: String` here? also, I may not get the purpose of `final` here since `val` is already immutable but would it be simpler to declare the object as `final`?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-4588] ML Attributes

Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:

    https://github.com/apache/spark/pull/4925#discussion_r25939258
  
    --- Diff: mllib/src/main/scala/org/apache/spark/ml/attribute/attributes.scala ---
    @@ -0,0 +1,512 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *    http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.spark.ml.attribute
    +
    +import scala.annotation.varargs
    +
    +import org.apache.spark.sql.types.{DoubleType, Metadata, MetadataBuilder, StructField}
    +
    +/**
    + * Abstract class for ML attributes.
    + */
    +sealed abstract class Attribute extends Serializable {
    +
    +  name.foreach { n =>
    +    require(n.nonEmpty, "Cannot have an empty string for name.")
    +  }
    +  index.foreach { i =>
    +    require(i >= 0, s"Index cannot be negative but got $i")
    +  }
    +
    +  /** Attribute type. */
    +  def attrType: AttributeType
    +
    +  /** Name of the attribute. None if it is not set. */
    +  def name: Option[String]
    +
    +  /** Copy with a new name. */
    +  def withName(name: String): Attribute
    +
    +  /** Copy without the name. */
    +  def withoutName: Attribute
    +
    +  /** Index of the attribute. None if it is not set. */
    +  def index: Option[Int]
    +
    +  /** Copy with a new index. */
    +  def withIndex(index: Int): Attribute
    +
    +  /** Copy without the index. */
    +  def withoutIndex: Attribute
    +
    +  /**
    +   * Tests whether this attribute is numeric, true for [[NumericAttribute]] and [[BinaryAttribute]].
    +   */
    +  def isNumeric: Boolean
    +
    +  /**
    +   * Tests whether this attribute is nominal, true for [[NominalAttribute]] and [[BinaryAttribute]].
    +   */
    +  def isNominal: Boolean
    +
    +  /**
    +   * Converts this attribute to [[Metadata]].
    +   * @param withType whether to include the type info
    +   */
    +  private[attribute] def toMetadata(withType: Boolean): Metadata
    +
    +  /**
    +   * Converts this attribute to [[Metadata]]. For numeric attributes, the type info is excluded to
    +   * save space, because numeric type is the default attribute type. For nominal and binary
    +   * attributes, the type info is included.
    +   */
    +  private[attribute] def toMetadata(): Metadata = {
    +    if (attrType == AttributeType.Numeric) {
    +      toMetadata(withType = false)
    +    } else {
    +      toMetadata(withType = true)
    +    }
    +  }
    +
    +  /**
    +   * Converts to a [[StructField]] with some existing metadata.
    +   * @param existingMetadata existing metadata to carry over
    +   */
    +  def toStructField(existingMetadata: Metadata): StructField = {
    +    val newMetadata = new MetadataBuilder()
    +      .withMetadata(existingMetadata)
    +      .putMetadata(AttributeKeys.ML_ATTR, withoutName.withoutIndex.toMetadata())
    +      .build()
    +    StructField(name.get, DoubleType, nullable = false, newMetadata)
    +  }
    +
    +  /** Converts to a [[StructField]]. */
    +  def toStructField(): StructField = toStructField(Metadata.empty)
    +
    +  override def toString: String = toMetadata(withType = true).toString
    +}
    +
    +/** Trait for ML attribute factories. */
    +private[attribute] trait AttributeFactory {
    +
    +  /**
    +   * Creates an [[Attribute]] from a [[Metadata]] instance.
    +   */
    +  private[attribute] def fromMetadata(metadata: Metadata): Attribute
    +
    +  /**
    +   * Creates an [[Attribute]] from a [[StructField]] instance.
    +   */
    +  def fromStructField(field: StructField): Attribute = {
    +    require(field.dataType == DoubleType)
    +    fromMetadata(field.metadata.getMetadata(AttributeKeys.ML_ATTR)).withName(field.name)
    +  }
    +}
    +
    +object Attribute extends AttributeFactory {
    +
    +  private[attribute] override def fromMetadata(metadata: Metadata): Attribute = {
    +    import org.apache.spark.ml.attribute.AttributeKeys._
    +    val attrType = if (metadata.contains(TYPE)) {
    +      metadata.getString(TYPE)
    +    } else {
    +      AttributeType.Numeric.name
    +    }
    +    getFactory(attrType).fromMetadata(metadata)
    +  }
    +
    +  /** Gets the attribute factory given the attribute type name. */
    +  private def getFactory(attrType: String): AttributeFactory = {
    +    if (attrType == AttributeType.Numeric.name) {
    +      NumericAttribute
    +    } else if (attrType == AttributeType.Nominal.name) {
    +      NominalAttribute
    +    } else if (attrType == AttributeType.Binary.name) {
    +      BinaryAttribute
    +    } else {
    +      throw new IllegalArgumentException(s"Cannot recognize type $attrType.")
    +    }
    +  }
    +}
    +
    +
    +/**
    + * A numeric attribute with optional summary statistics.
    + * @param name optional name
    + * @param index optional index
    + * @param min optional min value
    + * @param max optional max value
    + * @param std optional standard deviation
    + * @param sparsity optional sparsity (ratio of zeros)
    + */
    +class NumericAttribute private[ml] (
    +    override val name: Option[String] = None,
    +    override val index: Option[Int] = None,
    +    val min: Option[Double] = None,
    +    val max: Option[Double] = None,
    +    val std: Option[Double] = None,
    +    val sparsity: Option[Double] = None) extends Attribute {
    +
    +  std.foreach { s =>
    +    require(s >= 0.0, s"Standard deviation cannot be negative but got $s.")
    +  }
    +  sparsity.foreach { s =>
    +    require(s >= 0.0 && s <= 1.0, s"Sparsity must be in [0, 1] but got $s.")
    +  }
    +
    +  override def attrType: AttributeType = AttributeType.Numeric
    +
    +  override def withName(name: String): NumericAttribute = copy(name = Some(name))
    +  override def withoutName: NumericAttribute = copy(name = None)
    +
    +  override def withIndex(index: Int): NumericAttribute = copy(index = Some(index))
    +  override def withoutIndex: NumericAttribute = copy(index = None)
    +
    +  /** Copy with a new min value. */
    +  def withMin(min: Double): NumericAttribute = copy(min = Some(min))
    +
    +  /** Copy without the min value. */
    +  def withoutMin: NumericAttribute = copy(min = None)
    +
    +
    +  /** Copy with a new max value. */
    +  def withMax(max: Double): NumericAttribute = copy(max = Some(max))
    +
    +  /** Copy without the max value. */
    +  def withoutMax: NumericAttribute = copy(max = None)
    +
    +  /** Copy with a new standard deviation. */
    +  def withStd(std: Double): NumericAttribute = copy(std = Some(std))
    +
    +  /** Copy without the standard deviation. */
    +  def withoutStd: NumericAttribute = copy(std = None)
    +
    +  /** Copy with a new sparsity. */
    +  def withSparsity(sparsity: Double): NumericAttribute = copy(sparsity = Some(sparsity))
    +
    +  /** Copy without the sparsity. */
    +  def withoutSparsity: NumericAttribute = copy(sparsity = None)
    +
    +  /** Copy without summary statistics. */
    +  def withoutSummary: NumericAttribute = copy(min = None, max = None, std = None, sparsity = None)
    +
    +  override def isNumeric: Boolean = true
    +
    +  override def isNominal: Boolean = false
    +
    +  /** Convert this attribute to metadata. */
    +  private[attribute] override def toMetadata(withType: Boolean): Metadata = {
    +    import org.apache.spark.ml.attribute.AttributeKeys._
    +    val bldr = new MetadataBuilder()
    +    if (withType) bldr.putString(TYPE, attrType.name)
    +    name.foreach(bldr.putString(NAME, _))
    +    index.foreach(bldr.putLong(INDEX, _))
    +    min.foreach(bldr.putDouble(MIN, _))
    +    max.foreach(bldr.putDouble(MAX, _))
    +    std.foreach(bldr.putDouble(STD, _))
    +    sparsity.foreach(bldr.putDouble(SPARSITY, _))
    +    bldr.build()
    +  }
    +
    +  /** Creates a copy of this attribute with optional changes. */
    +  private def copy(
    +      name: Option[String] = name,
    +      index: Option[Int] = index,
    +      min: Option[Double] = min,
    +      max: Option[Double] = max,
    +      std: Option[Double] = std,
    +      sparsity: Option[Double] = sparsity): NumericAttribute = {
    +    new NumericAttribute(name, index, min, max, std, sparsity)
    +  }
    +
    +  override def equals(other: Any): Boolean = {
    +    other match {
    +      case o: NumericAttribute =>
    +        (name == o.name) &&
    +          (index == o.index) &&
    +          (min == o.min) &&
    +          (max == o.max) &&
    +          (std == o.std) &&
    +          (sparsity == o.sparsity)
    +      case _ =>
    +        false
    +    }
    +  }
    +
    +  override def hashCode: Int = {
    +    var sum = 17
    +    sum = 37 * sum + name.hashCode
    +    sum = 37 * sum + index.hashCode
    +    sum = 37 * sum + min.hashCode
    +    sum = 37 * sum + max.hashCode
    +    sum = 37 * sum + std.hashCode
    +    sum = 37 * sum + sparsity.hashCode
    +    sum
    +  }
    +}
    +
    +/**
    + * Factory methods for numeric attributes.
    + */
    +object NumericAttribute extends AttributeFactory {
    +
    +  /** The default numeric attribute. */
    +  val defaultAttr: NumericAttribute = new NumericAttribute
    +
    +  private[attribute] override def fromMetadata(metadata: Metadata): NumericAttribute = {
    +    import org.apache.spark.ml.attribute.AttributeKeys._
    +    val name = if (metadata.contains(NAME)) Some(metadata.getString(NAME)) else None
    +    val index = if (metadata.contains(INDEX)) Some(metadata.getLong(INDEX).toInt) else None
    +    val min = if (metadata.contains(MIN)) Some(metadata.getDouble(MIN)) else None
    +    val max = if (metadata.contains(MAX)) Some(metadata.getDouble(MAX)) else None
    +    val std = if (metadata.contains(STD)) Some(metadata.getDouble(STD)) else None
    +    val sparsity = if (metadata.contains(SPARSITY)) Some(metadata.getDouble(SPARSITY)) else None
    +    new NumericAttribute(name, index, min, max, std, sparsity)
    +  }
    +}
    +
    +/**
    + * A nominal attribute.
    + * @param name optional name
    + * @param index optional index
    + * @param isOrdinal whether this attribute is ordinal (optional)
    + * @param numValues optional number of values. At most one of `numValues` and `values` can be
    + *                  defined.
    + * @param values optional values. At most one of `numValues` and `values` can be defined.
    + */
    +class NominalAttribute private[ml] (
    +    override val name: Option[String] = None,
    +    override val index: Option[Int] = None,
    +    val isOrdinal: Option[Boolean] = None,
    +    val numValues: Option[Int] = None,
    +    val values: Option[Array[String]] = None) extends Attribute {
    +
    +  numValues.foreach { n =>
    +    require(n >= 0, s"numValues cannot be negative but got $n.")
    +  }
    +  require(!(numValues.isDefined && values.isDefined),
    +    "Cannot have both numValues and values defined.")
    +
    +  override def attrType: AttributeType = AttributeType.Nominal
    +
    +  override def isNumeric: Boolean = false
    +
    +  override def isNominal: Boolean = true
    +
    +  private lazy val valueToIndex: Map[String, Int] = {
    +    values.map(_.zipWithIndex.toMap).getOrElse(Map.empty)
    +  }
    +
    +  /** Index of a specific value. */
    +  def indexOf(value: String): Int = {
    +    valueToIndex(value)
    +  }
    +
    +  /** Tests whether this attribute contains a specific value. */
    +  def hasValue(value: String): Boolean = valueToIndex.contains(value)
    +
    +  /** Gets a value given its index. */
    +  def getValue(index: Int): String = values.get(index)
    +
    +  override def withName(name: String): NominalAttribute = copy(name = Some(name))
    +  override def withoutName: NominalAttribute = copy(name = None)
    +
    +  override def withIndex(index: Int): NominalAttribute = copy(index = Some(index))
    +  override def withoutIndex: NominalAttribute = copy(index = None)
    +
    +  /** Copy with new values and empty `numValues`. */
    +  def withValues(values: Array[String]): NominalAttribute = {
    +    copy(numValues = None, values = Some(values))
    +  }
    +
    +  /** Copy with new values and empty `numValues`. */
    +  @varargs
    +  def withValues(first: String, others: String*): NominalAttribute = {
    +    copy(numValues = None, values = Some((first +: others).toArray))
    +  }
    +
    +  /** Copy without the values. */
    +  def withoutValues: NominalAttribute = {
    +    copy(values = None)
    +  }
    +
    +  /** Copy with a new `numValues` and empty `values`. */
    +  def withNumValues(numValues: Int): NominalAttribute = {
    +    copy(numValues = Some(numValues), values = None)
    +  }
    +
    +  /** Copy without the `numValues`. */
    +  def withoutNumValues: NominalAttribute = copy(numValues = None)
    +
    +  /** Creates a copy of this attribute with optional changes. */
    +  private def copy(
    +      name: Option[String] = name,
    +      index: Option[Int] = index,
    +      isOrdinal: Option[Boolean] = isOrdinal,
    +      numValues: Option[Int] = numValues,
    +      values: Option[Array[String]] = values): NominalAttribute = {
    +    new NominalAttribute(name, index, isOrdinal, numValues, values)
    +  }
    +
    +  private[attribute] override def toMetadata(withType: Boolean): Metadata = {
    +    import org.apache.spark.ml.attribute.AttributeKeys._
    +    val bldr = new MetadataBuilder()
    +    if (withType) bldr.putString(TYPE, attrType.name)
    +    name.foreach(bldr.putString(NAME, _))
    +    index.foreach(bldr.putLong(INDEX, _))
    +    isOrdinal.foreach(bldr.putBoolean(ORDINAL, _))
    +    numValues.foreach(bldr.putLong(CARDINALITY, _))
    +    values.foreach(v => bldr.putStringArray(VALUES, v))
    +    bldr.build()
    +  }
    +
    +  override def equals(other: Any): Boolean = {
    +    other match {
    +      case o: NominalAttribute =>
    +        (name == o.name) &&
    +          (index == o.index) &&
    +          (isOrdinal == o.isOrdinal) &&
    +          (numValues == o.numValues) &&
    +          (values.map(_.toSeq) == o.values.map(_.toSeq))
    +      case _ =>
    +        false
    +    }
    +  }
    +
    +  override def hashCode: Int = {
    +    var sum = 17
    +    sum = 37 * sum + name.hashCode
    +    sum = 37 * sum + index.hashCode
    +    sum = 37 * sum + isOrdinal.hashCode
    +    sum = 37 * sum + numValues.hashCode
    +    sum = 37 * sum + values.map(_.toSeq).hashCode
    +    sum
    +  }
    +}
    +
    +/** Factory methods for nominal attributes. */
    +object NominalAttribute extends AttributeFactory {
    +
    +  /** The default nominal attribute. */
    +  final val defaultAttr: NominalAttribute = new NominalAttribute
    +
    +  private[attribute] override def fromMetadata(metadata: Metadata): NominalAttribute = {
    +    import org.apache.spark.ml.attribute.AttributeKeys._
    +    val name = if (metadata.contains(NAME)) Some(metadata.getString(NAME)) else None
    +    val index = if (metadata.contains(INDEX)) Some(metadata.getLong(INDEX).toInt) else None
    +    val isOrdinal = if (metadata.contains(ORDINAL)) Some(metadata.getBoolean(ORDINAL)) else None
    +    val cardinality =
    +      if (metadata.contains(CARDINALITY)) Some(metadata.getLong(CARDINALITY).toInt) else None
    +    val values =
    +      if (metadata.contains(VALUES)) Some(metadata.getStringArray(VALUES)) else None
    +    new NominalAttribute(name, index, isOrdinal, cardinality, values)
    +  }
    +}
    +
    +/**
    + * A binary attribute.
    + * @param name optional name
    + * @param index optional index
    + * @param values optionla values. If set, its size must be 2.
    + */
    +class BinaryAttribute private[ml] (
    --- End diff --
    
    So, a `BinaryAttribute` is not a subclass of `NominalAttribute` here, for example. That would be lovely in the sense that algorithms could transparently deal with `NominalAttribute` and automatically handle binary. I struggled to declare it in a sensible way though in my PR. Do you think it's possible to get that back?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-4588] ML Attributes

Posted by jkbradley <gi...@git.apache.org>.
Github user jkbradley commented on a diff in the pull request:

    https://github.com/apache/spark/pull/4925#discussion_r26061605
  
    --- Diff: mllib/src/main/scala/org/apache/spark/ml/attribute/package.scala ---
    @@ -0,0 +1,44 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *    http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.spark.ml
    +
    +import org.apache.spark.sql.DataFrame
    +import org.apache.spark.ml.attribute.{Attribute, AttributeGroup}
    +
    +/**
    + * ==ML attributes==
    + *
    + * The ML pipeline API uses [[DataFrame]]s as ML datasets.
    + * Each dataset consists of typed columns, e.g., string, double, vector, etc.
    + * However, knowing only the column type may not be sufficient to handle the data properly.
    + * For instance, a double column with values 0.0, 1.0, 2.0, ... may represent some label indices,
    + * which cannot be treated as numeric values in ML algorithms, and, for another instance, we may
    + * want to know the names and types of features stored in a vector column.
    + * ML attributes are used to provide additional information to describe columns in a dataset.
    + *
    + * ===ML columns===
    + *
    + * A column with ML attributes attached is called an ML column.
    + * The data in ML columns are stored as double values, i.e., an ML column is either a scalar column
    + * of double values or a vector column.
    + * Columns of other types must be encoded into ML columns using transformers.
    + * We use [[Attribute]] to describe a scalar ML column, and [[AttributeGroup]] to describe a vector
    + * ML column.
    + * ML attributes are stored in the metadata field of the column schema.
    + */
    +package object attribute
    --- End diff --
    
    Is there any way to make this doc show up in the Java docs?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-4588] ML Attributes

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/4925#issuecomment-78452427
  
      [Test build #28505 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/28505/consoleFull) for   PR 4925 at commit [`71d1bd0`](https://github.com/apache/spark/commit/71d1bd042360376b7f8f75ba6be3471f443ce34e).
     * This patch **passes all tests**.
     * This patch merges cleanly.
     * This patch adds the following public classes _(experimental)_:
      * `sealed abstract class AttributeType(val name: String)`
      * `sealed abstract class Attribute extends Serializable `



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-4588] ML Attributes

Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:

    https://github.com/apache/spark/pull/4925#discussion_r25985506
  
    --- Diff: mllib/src/main/scala/org/apache/spark/ml/attribute/attributes.scala ---
    @@ -0,0 +1,512 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *    http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.spark.ml.attribute
    +
    +import scala.annotation.varargs
    +
    +import org.apache.spark.sql.types.{DoubleType, Metadata, MetadataBuilder, StructField}
    +
    +/**
    + * Abstract class for ML attributes.
    + */
    +sealed abstract class Attribute extends Serializable {
    +
    +  name.foreach { n =>
    +    require(n.nonEmpty, "Cannot have an empty string for name.")
    +  }
    +  index.foreach { i =>
    +    require(i >= 0, s"Index cannot be negative but got $i")
    +  }
    +
    +  /** Attribute type. */
    +  def attrType: AttributeType
    +
    +  /** Name of the attribute. None if it is not set. */
    +  def name: Option[String]
    +
    +  /** Copy with a new name. */
    +  def withName(name: String): Attribute
    +
    +  /** Copy without the name. */
    +  def withoutName: Attribute
    +
    +  /** Index of the attribute. None if it is not set. */
    +  def index: Option[Int]
    +
    +  /** Copy with a new index. */
    +  def withIndex(index: Int): Attribute
    +
    +  /** Copy without the index. */
    +  def withoutIndex: Attribute
    +
    +  /**
    +   * Tests whether this attribute is numeric, true for [[NumericAttribute]] and [[BinaryAttribute]].
    +   */
    +  def isNumeric: Boolean
    +
    +  /**
    +   * Tests whether this attribute is nominal, true for [[NominalAttribute]] and [[BinaryAttribute]].
    +   */
    +  def isNominal: Boolean
    +
    +  /**
    +   * Converts this attribute to [[Metadata]].
    +   * @param withType whether to include the type info
    +   */
    +  private[attribute] def toMetadata(withType: Boolean): Metadata
    +
    +  /**
    +   * Converts this attribute to [[Metadata]]. For numeric attributes, the type info is excluded to
    +   * save space, because numeric type is the default attribute type. For nominal and binary
    +   * attributes, the type info is included.
    +   */
    +  private[attribute] def toMetadata(): Metadata = {
    +    if (attrType == AttributeType.Numeric) {
    +      toMetadata(withType = false)
    +    } else {
    +      toMetadata(withType = true)
    +    }
    +  }
    +
    +  /**
    +   * Converts to a [[StructField]] with some existing metadata.
    +   * @param existingMetadata existing metadata to carry over
    +   */
    +  def toStructField(existingMetadata: Metadata): StructField = {
    +    val newMetadata = new MetadataBuilder()
    +      .withMetadata(existingMetadata)
    +      .putMetadata(AttributeKeys.ML_ATTR, withoutName.withoutIndex.toMetadata())
    +      .build()
    +    StructField(name.get, DoubleType, nullable = false, newMetadata)
    +  }
    +
    +  /** Converts to a [[StructField]]. */
    +  def toStructField(): StructField = toStructField(Metadata.empty)
    +
    +  override def toString: String = toMetadata(withType = true).toString
    +}
    +
    +/** Trait for ML attribute factories. */
    +private[attribute] trait AttributeFactory {
    +
    +  /**
    +   * Creates an [[Attribute]] from a [[Metadata]] instance.
    +   */
    +  private[attribute] def fromMetadata(metadata: Metadata): Attribute
    +
    +  /**
    +   * Creates an [[Attribute]] from a [[StructField]] instance.
    +   */
    +  def fromStructField(field: StructField): Attribute = {
    +    require(field.dataType == DoubleType)
    +    fromMetadata(field.metadata.getMetadata(AttributeKeys.ML_ATTR)).withName(field.name)
    +  }
    +}
    +
    +object Attribute extends AttributeFactory {
    +
    +  private[attribute] override def fromMetadata(metadata: Metadata): Attribute = {
    +    import org.apache.spark.ml.attribute.AttributeKeys._
    +    val attrType = if (metadata.contains(TYPE)) {
    +      metadata.getString(TYPE)
    +    } else {
    +      AttributeType.Numeric.name
    +    }
    +    getFactory(attrType).fromMetadata(metadata)
    +  }
    +
    +  /** Gets the attribute factory given the attribute type name. */
    +  private def getFactory(attrType: String): AttributeFactory = {
    +    if (attrType == AttributeType.Numeric.name) {
    +      NumericAttribute
    +    } else if (attrType == AttributeType.Nominal.name) {
    +      NominalAttribute
    +    } else if (attrType == AttributeType.Binary.name) {
    +      BinaryAttribute
    +    } else {
    +      throw new IllegalArgumentException(s"Cannot recognize type $attrType.")
    +    }
    +  }
    +}
    +
    +
    +/**
    + * A numeric attribute with optional summary statistics.
    + * @param name optional name
    + * @param index optional index
    + * @param min optional min value
    + * @param max optional max value
    + * @param std optional standard deviation
    + * @param sparsity optional sparsity (ratio of zeros)
    + */
    +class NumericAttribute private[ml] (
    +    override val name: Option[String] = None,
    +    override val index: Option[Int] = None,
    +    val min: Option[Double] = None,
    +    val max: Option[Double] = None,
    +    val std: Option[Double] = None,
    +    val sparsity: Option[Double] = None) extends Attribute {
    +
    +  std.foreach { s =>
    +    require(s >= 0.0, s"Standard deviation cannot be negative but got $s.")
    +  }
    +  sparsity.foreach { s =>
    +    require(s >= 0.0 && s <= 1.0, s"Sparsity must be in [0, 1] but got $s.")
    +  }
    +
    +  override def attrType: AttributeType = AttributeType.Numeric
    +
    +  override def withName(name: String): NumericAttribute = copy(name = Some(name))
    +  override def withoutName: NumericAttribute = copy(name = None)
    +
    +  override def withIndex(index: Int): NumericAttribute = copy(index = Some(index))
    +  override def withoutIndex: NumericAttribute = copy(index = None)
    +
    +  /** Copy with a new min value. */
    +  def withMin(min: Double): NumericAttribute = copy(min = Some(min))
    +
    +  /** Copy without the min value. */
    +  def withoutMin: NumericAttribute = copy(min = None)
    +
    +
    +  /** Copy with a new max value. */
    +  def withMax(max: Double): NumericAttribute = copy(max = Some(max))
    +
    +  /** Copy without the max value. */
    +  def withoutMax: NumericAttribute = copy(max = None)
    +
    +  /** Copy with a new standard deviation. */
    +  def withStd(std: Double): NumericAttribute = copy(std = Some(std))
    +
    +  /** Copy without the standard deviation. */
    +  def withoutStd: NumericAttribute = copy(std = None)
    +
    +  /** Copy with a new sparsity. */
    +  def withSparsity(sparsity: Double): NumericAttribute = copy(sparsity = Some(sparsity))
    +
    +  /** Copy without the sparsity. */
    +  def withoutSparsity: NumericAttribute = copy(sparsity = None)
    +
    +  /** Copy without summary statistics. */
    +  def withoutSummary: NumericAttribute = copy(min = None, max = None, std = None, sparsity = None)
    +
    +  override def isNumeric: Boolean = true
    +
    +  override def isNominal: Boolean = false
    +
    +  /** Convert this attribute to metadata. */
    +  private[attribute] override def toMetadata(withType: Boolean): Metadata = {
    +    import org.apache.spark.ml.attribute.AttributeKeys._
    +    val bldr = new MetadataBuilder()
    +    if (withType) bldr.putString(TYPE, attrType.name)
    +    name.foreach(bldr.putString(NAME, _))
    +    index.foreach(bldr.putLong(INDEX, _))
    +    min.foreach(bldr.putDouble(MIN, _))
    +    max.foreach(bldr.putDouble(MAX, _))
    +    std.foreach(bldr.putDouble(STD, _))
    +    sparsity.foreach(bldr.putDouble(SPARSITY, _))
    +    bldr.build()
    +  }
    +
    +  /** Creates a copy of this attribute with optional changes. */
    +  private def copy(
    +      name: Option[String] = name,
    +      index: Option[Int] = index,
    +      min: Option[Double] = min,
    +      max: Option[Double] = max,
    +      std: Option[Double] = std,
    +      sparsity: Option[Double] = sparsity): NumericAttribute = {
    +    new NumericAttribute(name, index, min, max, std, sparsity)
    +  }
    +
    +  override def equals(other: Any): Boolean = {
    +    other match {
    +      case o: NumericAttribute =>
    +        (name == o.name) &&
    +          (index == o.index) &&
    +          (min == o.min) &&
    +          (max == o.max) &&
    +          (std == o.std) &&
    +          (sparsity == o.sparsity)
    +      case _ =>
    +        false
    +    }
    +  }
    +
    +  override def hashCode: Int = {
    +    var sum = 17
    +    sum = 37 * sum + name.hashCode
    +    sum = 37 * sum + index.hashCode
    +    sum = 37 * sum + min.hashCode
    +    sum = 37 * sum + max.hashCode
    +    sum = 37 * sum + std.hashCode
    +    sum = 37 * sum + sparsity.hashCode
    +    sum
    +  }
    +}
    +
    +/**
    + * Factory methods for numeric attributes.
    + */
    +object NumericAttribute extends AttributeFactory {
    +
    +  /** The default numeric attribute. */
    +  val defaultAttr: NumericAttribute = new NumericAttribute
    +
    +  private[attribute] override def fromMetadata(metadata: Metadata): NumericAttribute = {
    +    import org.apache.spark.ml.attribute.AttributeKeys._
    +    val name = if (metadata.contains(NAME)) Some(metadata.getString(NAME)) else None
    +    val index = if (metadata.contains(INDEX)) Some(metadata.getLong(INDEX).toInt) else None
    +    val min = if (metadata.contains(MIN)) Some(metadata.getDouble(MIN)) else None
    +    val max = if (metadata.contains(MAX)) Some(metadata.getDouble(MAX)) else None
    +    val std = if (metadata.contains(STD)) Some(metadata.getDouble(STD)) else None
    +    val sparsity = if (metadata.contains(SPARSITY)) Some(metadata.getDouble(SPARSITY)) else None
    +    new NumericAttribute(name, index, min, max, std, sparsity)
    +  }
    +}
    +
    +/**
    + * A nominal attribute.
    + * @param name optional name
    + * @param index optional index
    + * @param isOrdinal whether this attribute is ordinal (optional)
    + * @param numValues optional number of values. At most one of `numValues` and `values` can be
    + *                  defined.
    + * @param values optional values. At most one of `numValues` and `values` can be defined.
    + */
    +class NominalAttribute private[ml] (
    +    override val name: Option[String] = None,
    +    override val index: Option[Int] = None,
    +    val isOrdinal: Option[Boolean] = None,
    +    val numValues: Option[Int] = None,
    +    val values: Option[Array[String]] = None) extends Attribute {
    +
    +  numValues.foreach { n =>
    +    require(n >= 0, s"numValues cannot be negative but got $n.")
    +  }
    +  require(!(numValues.isDefined && values.isDefined),
    +    "Cannot have both numValues and values defined.")
    +
    +  override def attrType: AttributeType = AttributeType.Nominal
    +
    +  override def isNumeric: Boolean = false
    +
    +  override def isNominal: Boolean = true
    +
    +  private lazy val valueToIndex: Map[String, Int] = {
    +    values.map(_.zipWithIndex.toMap).getOrElse(Map.empty)
    +  }
    +
    +  /** Index of a specific value. */
    +  def indexOf(value: String): Int = {
    +    valueToIndex(value)
    +  }
    +
    +  /** Tests whether this attribute contains a specific value. */
    +  def hasValue(value: String): Boolean = valueToIndex.contains(value)
    +
    +  /** Gets a value given its index. */
    +  def getValue(index: Int): String = values.get(index)
    +
    +  override def withName(name: String): NominalAttribute = copy(name = Some(name))
    +  override def withoutName: NominalAttribute = copy(name = None)
    +
    +  override def withIndex(index: Int): NominalAttribute = copy(index = Some(index))
    +  override def withoutIndex: NominalAttribute = copy(index = None)
    +
    +  /** Copy with new values and empty `numValues`. */
    +  def withValues(values: Array[String]): NominalAttribute = {
    +    copy(numValues = None, values = Some(values))
    +  }
    +
    +  /** Copy with new values and empty `numValues`. */
    +  @varargs
    +  def withValues(first: String, others: String*): NominalAttribute = {
    +    copy(numValues = None, values = Some((first +: others).toArray))
    +  }
    +
    +  /** Copy without the values. */
    +  def withoutValues: NominalAttribute = {
    +    copy(values = None)
    +  }
    +
    +  /** Copy with a new `numValues` and empty `values`. */
    +  def withNumValues(numValues: Int): NominalAttribute = {
    +    copy(numValues = Some(numValues), values = None)
    +  }
    +
    +  /** Copy without the `numValues`. */
    +  def withoutNumValues: NominalAttribute = copy(numValues = None)
    +
    +  /** Creates a copy of this attribute with optional changes. */
    +  private def copy(
    +      name: Option[String] = name,
    +      index: Option[Int] = index,
    +      isOrdinal: Option[Boolean] = isOrdinal,
    +      numValues: Option[Int] = numValues,
    +      values: Option[Array[String]] = values): NominalAttribute = {
    +    new NominalAttribute(name, index, isOrdinal, numValues, values)
    +  }
    +
    +  private[attribute] override def toMetadata(withType: Boolean): Metadata = {
    +    import org.apache.spark.ml.attribute.AttributeKeys._
    +    val bldr = new MetadataBuilder()
    +    if (withType) bldr.putString(TYPE, attrType.name)
    +    name.foreach(bldr.putString(NAME, _))
    +    index.foreach(bldr.putLong(INDEX, _))
    +    isOrdinal.foreach(bldr.putBoolean(ORDINAL, _))
    +    numValues.foreach(bldr.putLong(CARDINALITY, _))
    +    values.foreach(v => bldr.putStringArray(VALUES, v))
    +    bldr.build()
    +  }
    +
    +  override def equals(other: Any): Boolean = {
    +    other match {
    +      case o: NominalAttribute =>
    +        (name == o.name) &&
    +          (index == o.index) &&
    +          (isOrdinal == o.isOrdinal) &&
    +          (numValues == o.numValues) &&
    +          (values.map(_.toSeq) == o.values.map(_.toSeq))
    +      case _ =>
    +        false
    +    }
    +  }
    +
    +  override def hashCode: Int = {
    +    var sum = 17
    +    sum = 37 * sum + name.hashCode
    +    sum = 37 * sum + index.hashCode
    +    sum = 37 * sum + isOrdinal.hashCode
    +    sum = 37 * sum + numValues.hashCode
    +    sum = 37 * sum + values.map(_.toSeq).hashCode
    +    sum
    +  }
    +}
    +
    +/** Factory methods for nominal attributes. */
    +object NominalAttribute extends AttributeFactory {
    +
    +  /** The default nominal attribute. */
    +  final val defaultAttr: NominalAttribute = new NominalAttribute
    +
    +  private[attribute] override def fromMetadata(metadata: Metadata): NominalAttribute = {
    +    import org.apache.spark.ml.attribute.AttributeKeys._
    +    val name = if (metadata.contains(NAME)) Some(metadata.getString(NAME)) else None
    +    val index = if (metadata.contains(INDEX)) Some(metadata.getLong(INDEX).toInt) else None
    +    val isOrdinal = if (metadata.contains(ORDINAL)) Some(metadata.getBoolean(ORDINAL)) else None
    +    val cardinality =
    +      if (metadata.contains(CARDINALITY)) Some(metadata.getLong(CARDINALITY).toInt) else None
    +    val values =
    +      if (metadata.contains(VALUES)) Some(metadata.getStringArray(VALUES)) else None
    +    new NominalAttribute(name, index, isOrdinal, cardinality, values)
    +  }
    +}
    +
    +/**
    + * A binary attribute.
    + * @param name optional name
    + * @param index optional index
    + * @param values optionla values. If set, its size must be 2.
    + */
    +class BinaryAttribute private[ml] (
    --- End diff --
    
    Yeah it sounds nice in theory but it got complicated to express. Go ahead and see how it works.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-4588] ML Attributes

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/4925#issuecomment-77659647
  
      [Test build #28357 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/28357/consoleFull) for   PR 4925 at commit [`617be40`](https://github.com/apache/spark/commit/617be409e8a973a93c3ed6d9dcc1b345708e22e4).
     * This patch **passes all tests**.
     * This patch merges cleanly.
     * This patch adds the following public classes _(experimental)_:
      * `sealed abstract class AttributeType(val name: String)`
      * `sealed abstract class Attribute extends Serializable `



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-4588] ML Attributes

Posted by mengxr <gi...@git.apache.org>.
Github user mengxr commented on a diff in the pull request:

    https://github.com/apache/spark/pull/4925#discussion_r26062079
  
    --- Diff: mllib/src/main/scala/org/apache/spark/ml/attribute/package.scala ---
    @@ -0,0 +1,44 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *    http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.spark.ml
    +
    +import org.apache.spark.sql.DataFrame
    +import org.apache.spark.ml.attribute.{Attribute, AttributeGroup}
    +
    +/**
    + * ==ML attributes==
    + *
    + * The ML pipeline API uses [[DataFrame]]s as ML datasets.
    + * Each dataset consists of typed columns, e.g., string, double, vector, etc.
    + * However, knowing only the column type may not be sufficient to handle the data properly.
    + * For instance, a double column with values 0.0, 1.0, 2.0, ... may represent some label indices,
    + * which cannot be treated as numeric values in ML algorithms, and, for another instance, we may
    + * want to know the names and types of features stored in a vector column.
    + * ML attributes are used to provide additional information to describe columns in a dataset.
    + *
    + * ===ML columns===
    + *
    + * A column with ML attributes attached is called an ML column.
    + * The data in ML columns are stored as double values, i.e., an ML column is either a scalar column
    + * of double values or a vector column.
    + * Columns of other types must be encoded into ML columns using transformers.
    + * We use [[Attribute]] to describe a scalar ML column, and [[AttributeGroup]] to describe a vector
    + * ML column.
    + * ML attributes are stored in the metadata field of the column schema.
    + */
    +package object attribute
    --- End diff --
    
    We need `package-info.java` for Java. Let me include it with the next update.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-4588] ML Attributes

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/4925#issuecomment-77502236
  
      [Test build #28326 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/28326/consoleFull) for   PR 4925 at commit [`b1aceef`](https://github.com/apache/spark/commit/b1aceefaf9738e7528c179b43bfc99c793160164).
     * This patch merges cleanly.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-4588] ML Attributes

Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:

    https://github.com/apache/spark/pull/4925#discussion_r25939395
  
    --- Diff: mllib/src/main/scala/org/apache/spark/ml/attribute/AttributeGroup.scala ---
    @@ -0,0 +1,234 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *    http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.spark.ml.attribute
    +
    +import scala.collection.mutable.ArrayBuffer
    +
    +import org.apache.spark.mllib.linalg.VectorUDT
    +import org.apache.spark.sql.types.{Metadata, MetadataBuilder, StructField}
    +
    +/**
    + * Attributes that describe a vector ML column.
    + *
    + * @param name name of the attribute group (the ML column name)
    + * @param numAttributes optional number of attributes. At most one of `numAttributes` and `attrs`
    + *                      can be defined.
    + * @param attrs optional array of attributes. Attribute will be copied with their corresponding
    + *              indices in the array.
    + */
    +class AttributeGroup private (
    --- End diff --
    
    I'm probably overlooking this but how do I put a `AttributeGroup` inside another `AttributeGroup`? they are intended to be nest-able?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-4588] ML Attributes

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/4925#issuecomment-77597218
  
      [Test build #28349 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/28349/consoleFull) for   PR 4925 at commit [`393ffdc`](https://github.com/apache/spark/commit/393ffdca897b27e4bb263bb538395a045dbedf26).
     * This patch merges cleanly.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org