Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Forecast evaluator + SMAPE metric #337

Merged
merged 7 commits into from
Jun 22, 2019
Merged

Forecast evaluator + SMAPE metric #337

merged 7 commits into from
Jun 22, 2019

Conversation

wsuchy
Copy link
Contributor

@wsuchy wsuchy commented Jun 19, 2019

This is only a POC, as I would like to check if it gets (eventually) merged.
So far I've implemented only one metric (sMAPE) but ideally I would like to put all forecast related metrics there (including some already present like MAE, etc).

As we don't have any forecasting model yet I've used linear regression.

@codecov
Copy link

codecov bot commented Jun 19, 2019

Codecov Report

Merging #337 into master will decrease coverage by 0.01%.
The diff coverage is 90.9%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #337      +/-   ##
==========================================
- Coverage   86.65%   86.63%   -0.02%     
==========================================
  Files         334      335       +1     
  Lines       10752    10763      +11     
  Branches      565      344     -221     
==========================================
+ Hits         9317     9325       +8     
- Misses       1435     1438       +3
Impacted Files Coverage Δ
...m/salesforce/op/evaluators/EvaluationMetrics.scala 86.66% <100%> (+0.95%) ⬆️
...salesforce/op/evaluators/OpForecastEvaluator.scala 90% <90%> (ø)
.../com/salesforce/op/local/MLeapModelConverter.scala 5.12% <0%> (-5.13%) ⬇️

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 5b82f42...b1cf72b. Read the comment docs.


/**
*
* Instance to evaluate Regression metrics
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

docs are incorrect


val smape: Double = getSMAPE(dataUse, getLabelCol, getPredictionValueCol)
val metrics = ForecastMetrics(
sMAPE = smape
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reformat into one liner

}

// https://www.m4.unic.ac.cy/wp-content/uploads/2018/03/M4-Competitors-Guide.pdf
case class ReduceSMAPE(nominator: Double, denominator: Double, cnt: Long) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

instead of defining + you can use algebird macro for case classes, i.e.

import com.twitter.algebird.Operators._
import com.twitter.algebird.macros.caseclass

implicit val smapeSG = caseclass.semigroup[SMAPEValue]
case class SMAPEValue(nominator: Double, denominator: Double, cnt: Long)

}

object ReduceSMAPE {
def apply(y: Double, y_hat: Double): ReduceSMAPE = {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. please make this the only public ctor
  2. avoid _ in names, instead use camelCase

}

/**
* Metrics of Regression Problem
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

invalid docs

Copy link
Collaborator

@tovbinm tovbinm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@tovbinm tovbinm changed the title Forecast evaluator Forecast evaluator + SMAPE metric Jun 22, 2019
@tovbinm tovbinm merged commit e552147 into master Jun 22, 2019
@tovbinm tovbinm deleted the ks/forecastEvaluator branch June 22, 2019 16:00
This was referenced Jul 10, 2019
@salesforce-cla
Copy link

Thanks for the contribution! Before we can merge this, we need @wsuchy to sign the Salesforce.com Contributor License Agreement.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants