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.

Sign up for GitHub

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

Jump to bottom

Model combiner #385

Merged
merged 19 commits into from
Sep 3, 2019
Merged

Model combiner #385

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
046dbef
made model combiner
leahmcguire Aug 13, 2019
ba8e587
got class test passing
leahmcguire Aug 15, 2019
315744e
need to fix insights for multiple sanity checkers
leahmcguire Aug 16, 2019
6df8478
fixed model insights
leahmcguire Aug 19, 2019
2dfec2f
merged
leahmcguire Aug 19, 2019
82f130f
style
leahmcguire Aug 19, 2019
d3832ed
style
leahmcguire Aug 19, 2019
7690826
removed println
leahmcguire Aug 19, 2019
b39da7c
cleanup
leahmcguire Aug 19, 2019
4f28cca
minor fix
leahmcguire Aug 20, 2019
63bf74b
updated EinsteinAppArgs to be based on of MlAppArgs base class
leahmcguire Aug 22, 2019
5bf6299
added test for mismatched metrics
leahmcguire Aug 22, 2019
e57fe22
merged
leahmcguire Aug 22, 2019
4e36467
fixed custom names
leahmcguire Aug 22, 2019
96367d5
Merge branch 'master' into lm/modelCombiner
tovbinm Aug 27, 2019
712e84e
Merge branch 'master' of github.com:salesforce/TransmogrifAI into lm/…
leahmcguire Aug 29, 2019
6139bd0
adressing comments
leahmcguire Aug 29, 2019
7d27995
Merge branch 'lm/modelCombiner' of github.com:salesforce/TransmogrifA…
leahmcguire Aug 29, 2019
41c6654
Merge branch 'master' into lm/modelCombiner
leahmcguire Sep 3, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
removed println
  • Loading branch information
@leahmcguire
leahmcguire committed Aug 19, 2019
commit 7690826ab12af1d61bcf203a74d1cf081f8d729e
15 changes: 8 additions & 7 deletions core/src/main/scala/com/salesforce/op/stages/impl/selector/SelectedCombiner.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ trait SelectedCombinerParams extends Params {
isValid = (in: String) => CombinationStrategy.values.map(_.entryName).contains(in)
)
def setCombinationStrategy(value: CombinationStrategy): this.type = set(combinationStrategy, value.entryName)
def getCombinationStrategy(): CombinationStrategy = CombinationStrategy.withName($(combinationStrategy))
def getCombinationStrategy(): CombinationStrategy= CombinationStrategy.withName($(combinationStrategy))
setDefault(combinationStrategy, CombinationStrategy.Best.entryName)

}
Expand Down Expand Up @@ -163,7 +163,8 @@ class SelectedCombiner

val (metricValue1, metricValue2) = (getMet(metricValueOpt1), getMet(metricValueOpt2))

val (weight1, weight2) = getCombinationStrategy() match {
val strategy = getCombinationStrategy()
val (weight1, weight2) = strategy match {
case CombinationStrategy.Best =>
Copy link
Collaborator

Choose a reason for hiding this comment

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

this can be a method on CombinationStrategy itself. e.g. val (weight1, weight2) = getCombinationStrategy().computeWeights(metricValue1, metricValue2)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

it actually cant unless I move the EvalMetric class to the features module

if (metricValue1 > metricValue2) (1.0, 0.0) else (0.0, 1.0)
case CombinationStrategy.Weighted =>
Expand All @@ -173,7 +174,7 @@ class SelectedCombiner
}

val model: SelectedCombinerModel = new SelectedCombinerModel(
weight1 = weight1, weight2 = weight2, strategy = getCombinationStrategy(),
weight1 = weight1, weight2 = weight2, strategy = strategy,
operationName = operationName, uid = uid
)
.setEvaluators(evaluators)
Expand Down Expand Up @@ -226,11 +227,11 @@ final class SelectedCombinerModel private[op]
}


sealed abstract class CombinationStrategy(val name: String) extends EnumEntry with Serializable
sealed abstract class CombinationStrategy extends EnumEntry with Serializable

object CombinationStrategy extends Enum[CombinationStrategy] {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this enum should be defined in feature module and registered with the json in OpPipelineStageReadWriteFormats

Copy link
Contributor

Choose a reason for hiding this comment

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

Docs here too

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done

val values = findValues
case object Weighted extends CombinationStrategy("weighted")
case object Equal extends CombinationStrategy("equal")
case object Best extends CombinationStrategy("best")
case object Weighted extends CombinationStrategy
case object Equal extends CombinationStrategy
case object Best extends CombinationStrategy
}
1 change: 0 additions & 1 deletion core/src/test/scala/com/salesforce/op/ModelInsightsTest.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -806,7 +806,6 @@ class ModelInsightsTest extends FlatSpec with PassengerSparkFixtureTest with Dou
val workflowModel = new OpWorkflow().setResultFeatures(pred, predComb)
.setParameters(params).setReader(dataReader).train()
val insights = workflowModel.modelInsights(predComb)
println(insights.prettyPrint())
insights.selectedModelInfo.nonEmpty shouldBe true
insights.features.foreach(_.derivedFeatures.foreach(_.contribution shouldBe Seq()))
insights.features.map(_.featureName).toSet shouldBe
Expand Down