Circular date representations to default date transmogrifier #100

leahmcguire · 2018-08-28T22:29:56Z

Related issues
Adding circular date representations to default date transmogrifier

Describe the proposed solution
Added a DateMap circular transformer
Make Date circular transformer take a sequence instead of a single feature
Updated and added shortcuts for all Date and DateMap classes toUnitCircle and added default transformations to vectorize for all of these classes
Fixed metadata on Geolocation and GeolocationMap so that keep the name of the column in descriptorValue

Additional context
Current default is time ago from now but circular representations are better for capturing many business use cases

…circleDates

codecov · 2018-08-28T23:10:46Z

Codecov Report

Merging #100 into master will decrease coverage by 5.26%.
The diff coverage is 85.56%.

@@            Coverage Diff             @@
##           master     #100      +/-   ##
==========================================
- Coverage   91.34%   86.08%   -5.27%     
==========================================
  Files         131      297     +166     
  Lines        5234     9658    +4424     
  Branches      184      437     +253     
==========================================
+ Hits         4781     8314    +3533     
- Misses        453     1344     +891

Impacted Files	Coverage Δ
...om/salesforce/op/filters/FeatureDistribution.scala	`97.87% <ø> (ø)`	⬆️
...ala/com/salesforce/op/features/types/package.scala	`51.44% <0%> (ø)`
...lesforce/op/test/TestOpVectorMetadataBuilder.scala	`0% <0%> (ø)`
...rce/op/stages/OpPipelineStageReadWriteShared.scala	`100% <100%> (ø)`
.../scala/com/salesforce/op/dsl/RichDateFeature.scala	`100% <100%> (ø)`	⬆️
...sforce/op/stages/impl/feature/Transmogrifier.scala	`96.74% <100%> (+0.05%)`	⬆️
...s/impl/feature/DateMapToUnitCircleVectorizer.scala	`100% <100%> (ø)`
...stages/impl/feature/GeolocationMapVectorizer.scala	`100% <100%> (ø)`	⬆️
...com/salesforce/op/features/types/Geolocation.scala	`90.9% <100%> (ø)`
...m/salesforce/op/utils/spark/OpVectorMetadata.scala	`82.97% <100%> (ø)`
... and 169 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8ecb902...c860c02. Read the comment docs.

tovbinm · 2018-08-30T00:23:19Z

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

     * @return result feature of type Vector
     */
    def vectorize
    (
      dateListPivot: DateListPivot,
      referenceDate: JDateTime = TransmogrifierDefaults.ReferenceDate,
      trackNulls: Boolean = TransmogrifierDefaults.TrackNulls,
+      circularDateRepresentations: Seq[TimePeriod] = TransmogrifierDefaults.CircularDateRepresentations,


this is a ridiculously long param name ;) can we perhaps shorten to circularDateRep? (same elsewhere)

tovbinm · 2018-08-30T00:24:33Z

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

+      val timePeriods = circularDateRepresentations.map(tp => f.toUnitCircle(tp, others))
+      val time = f.toDateList().vectorize(dateListPivot = dateListPivot, referenceDate = referenceDate,
+        trackNulls = trackNulls, others = others.map(_.toDateList()))
+      (timePeriods :+ time).combine()


if (timePeriods.isEmpty) time else (timePeriods :+ time).combine() (same elsewhere)

tovbinm · 2018-08-30T00:29:22Z

core/src/main/scala/com/salesforce/op/stages/impl/feature/DateToUnitCircleTransformer.scala

 /**
 * Following: http://webspace.ship.edu/pgmarr/geo441/lectures/lec%2016%20-%20directional%20statistics.pdf
 * Transforms a Date or DateTime field into a cartesian coordinate representation
 * of an extracted time period on the unit circle
 *
- * @param timePeriod The time period to extract from the timestamp
+ * parameter timePeriod The time period to extract from the timestamp


do you mean spark parameter?

yes - it bugged me that it was highlighted red

tovbinm · 2018-08-30T00:31:16Z

core/src/main/scala/com/salesforce/op/stages/impl/feature/DateToUnitCircleTransformer.scala

+ * Monday is the first day of the week
+ * & the first week of the year is the week wit the first Monday after Jan 1.
+ */
+class DateMapToUnitCircleVectorizer[T <: DateMap]


please put this class into a separate file.

tovbinm · 2018-08-30T00:31:53Z

core/src/main/scala/com/salesforce/op/stages/impl/feature/DateToUnitCircleTransformer.scala

+    meta.withColumns(cols.toArray)
+  }
+
+  override def fitFn(dataset: Dataset[Seq[Map[String, Long]]]): SequenceModel[T, OPVector] = {


Dataset[Seq[T#Value]]

tovbinm · 2018-08-30T00:36:38Z

core/src/main/scala/com/salesforce/op/stages/impl/feature/DateToUnitCircleTransformer.scala

-    val history = Map(in1.name -> FeatureHistory(originFeatures = in1.originFeatures, stages = in1.stages))
-    setMetadata(OpVectorMetadata(getOutputFeatureName, columns, history).toMetadata)
+    radians.map(r => Array(math.cos(r), math.sin(r))).
+      getOrElse(Array(0.0, 0.0))


please reformat: redundant endline.

tovbinm · 2018-08-30T00:38:12Z

...src/test/scala/com/salesforce/op/stages/impl/feature/DateMapToUnitCircleVectorizerTest.scala

+    val fitted = estimator.fit(inputData)
+    val meta = OpVectorMetadata(fitted.getOutputFeatureName, fitted.getMetadata())
+    meta.columns.length shouldBe 4
+    meta.columns.map(_.grouping.get) shouldEqual Seq("a", "a", "b", "b")


meta.columns.flatMap(_.grouping) and meta.columns.flatMap(_.descriptorValue)

tovbinm · 2018-08-30T00:39:21Z

core/src/test/scala/com/salesforce/op/stages/impl/feature/DateToUnitCircleTransformerTest.scala

@@ -99,8 +99,8 @@ class DateToUnitCircleTransformerTest extends OpTransformerSpec[OPVector, DateTo
    val transformed = vectorizer.transform(ds)
    val meta = OpVectorMetadata(transformed.schema(vectorizer.getOutput().name))
    meta.columns.length should equal (2)
-    meta.columns(0).indicatorValue.get should equal("x_HourOfDay")
-    meta.columns(1).indicatorValue.get should equal("y_HourOfDay")
+    meta.columns(0).descriptorValue.get should equal("x_HourOfDay")


meta.columns(0).descriptorValue should be Some("x_HourOfDay")

tovbinm · 2018-08-30T00:39:54Z

core/src/test/scala/com/salesforce/op/stages/impl/feature/OPMapVectorizerTest.scala

@@ -46,6 +46,7 @@ import org.junit.runner.RunWith
 import org.scalatest.junit.JUnitRunner
 import org.scalatest.{FlatSpec, Matchers}
 import org.slf4j.LoggerFactory
+// import org.apache.log4j.Level


redundant import

tovbinm · 2018-08-30T00:41:12Z

features/src/main/scala/com/salesforce/op/features/types/package.scala

@@ -196,6 +196,13 @@ package object types extends FeatureTypeSparkConverters {
  implicit class VectorConversions(val v: Vector) extends AnyVal {
    def toOPVector: OPVector = new OPVector(v)
  }
+  // Arrays
+  implicit class ArrayDoubleConversions(val v: Array[Double]) extends AnyVal {
+    def toReal: Seq[Real] = v.map(_.toReal)


keep the Array type in return value, ie. def toReal: Array[Real] and def toRealNN: Array[RealNN]

tovbinm · 2018-08-30T00:43:23Z

@leahmcguire can you please update the description of the PR shortcuts & vectorizers were added (we will use it in our release notes later).

…into lm/circleDates

tovbinm

lgtm!!

salesforce-cla · 2021-03-07T14:27:53Z

Thanks for the contribution! Unfortunately we can't verify the commit author(s): leahmcguire <l***@s***.com>. One possible solution is to add that email to your GitHub account. Alternatively you can change your commits to another email and force push the change. After getting your commits associated with your GitHub account, refresh the status of this Pull Request.

leahmcguire added 7 commits August 23, 2018 09:24

added circular transform to date vectorizer default

1ece9f1

Merge branch 'master' of github.com:salesforce/TransmogrifAI into lm/…

0fbadf1

…circleDates

added date map unit circle transformer

859a9e7

Merge branch 'master' of github.com:salesforce/TransmogrifAI into lm/…

d13aa56

…circleDates

fixed tests

ab4e9ec

fixed everythign but geolocation test

b4847d4

fixed tests

e039667

leahmcguire requested a review from tovbinm as a code owner August 28, 2018 22:29

Merge branch 'master' into lm/circleDates

a064dbc

tovbinm changed the title ~~Lm/circle dates~~ Circle dates Aug 28, 2018

tovbinm changed the title ~~Circle dates~~ Circular date representations to default date transmogrifier Aug 28, 2018

leahmcguire requested review from kinfaikan, Jauntbox and sxd929 August 29, 2018 03:32

leahmcguire added the ready for review label Aug 29, 2018

Merge branch 'master' into lm/circleDates

b9eda5e

kinfaikan approved these changes Aug 30, 2018

View reviewed changes

tovbinm reviewed Aug 30, 2018

View reviewed changes

tovbinm and others added 5 commits August 30, 2018 13:59

Merge branch 'master' into lm/circleDates

d4eb151

Merge branch 'master' into lm/circleDates

afaf737

Merge branch 'master' into lm/circleDates

51fcf2a

fixing comments

6de3d15

Merge branch 'lm/circleDates' of github.com:salesforce/TransmogrifAI …

c860c02

…into lm/circleDates

tovbinm approved these changes Aug 30, 2018

View reviewed changes

tovbinm merged commit 52341f9 into master Aug 30, 2018

tovbinm deleted the lm/circleDates branch August 30, 2018 23:42

ericwayman pushed a commit that referenced this pull request Feb 8, 2019

Circular date representations to default date transmogrifier (#100)

dad535d

salesforce-cla bot added the cla:signed label Jul 30, 2020

salesforce-cla bot added cla:missing and removed cla:signed labels Mar 7, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Circular date representations to default date transmogrifier #100

Circular date representations to default date transmogrifier #100

leahmcguire commented Aug 28, 2018 •

edited

Loading

codecov bot commented Aug 28, 2018 •

edited

Loading

tovbinm Aug 30, 2018

tovbinm Aug 30, 2018

tovbinm Aug 30, 2018

leahmcguire Aug 30, 2018

tovbinm Aug 30, 2018

tovbinm Aug 30, 2018

tovbinm Aug 30, 2018

tovbinm Aug 30, 2018

tovbinm Aug 30, 2018

tovbinm Aug 30, 2018

tovbinm Aug 30, 2018

tovbinm commented Aug 30, 2018

tovbinm left a comment

salesforce-cla bot commented Mar 7, 2021

Circular date representations to default date transmogrifier #100

Circular date representations to default date transmogrifier #100

Conversation

leahmcguire commented Aug 28, 2018 • edited Loading

codecov bot commented Aug 28, 2018 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tovbinm commented Aug 30, 2018

tovbinm left a comment

Choose a reason for hiding this comment

salesforce-cla bot commented Mar 7, 2021

leahmcguire commented Aug 28, 2018 •

edited

Loading

codecov bot commented Aug 28, 2018 •

edited

Loading