You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by thvasilo <gi...@git.apache.org> on 2015/06/30 15:07:33 UTC
[GitHub] flink pull request: [FLINK-2297] [ml] Add threshold setting for SV...
GitHub user thvasilo opened a pull request:
https://github.com/apache/flink/pull/874
[FLINK-2297] [ml] Add threshold setting for SVM binary predictions
Currently SVM outputs the raw decision function values when using the predict function.
We should have instead the ability to set a threshold above which examples are labeled as positive (1.0) and below negative (-1.0). Then the prediction function can be directly used for evaluation.
This provides that functionality, as well as the ability to provide the raw decision function values.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/thvasilo/flink svm-threshold
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/flink/pull/874.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 #874
----
commit 279f12a6031f9d7b73e6b69303ae44627df4c401
Author: Theodore Vasiloudis <tv...@sics.se>
Date: 2015-06-30T12:49:03Z
Added parameters argument to PredictOperation predict function.
commit 1cea7a9a71ba04b6a9433789c0787112c7a24084
Author: Theodore Vasiloudis <tv...@sics.se>
Date: 2015-06-30T12:59:57Z
Made the type parameter for Parameter to be covariant.
commit 00cf902d1481d698648abe6f037583d8d543fa53
Author: Theodore Vasiloudis <tv...@sics.se>
Date: 2015-06-30T13:00:49Z
Added Threshold option for SVM, to determine which predictions are positive/negative.
----
---
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.
---
[GitHub] flink pull request: [FLINK-2297] [ml] Add threshold setting for SV...
Posted by thvasilo <gi...@git.apache.org>.
Github user thvasilo commented on a diff in the pull request:
https://github.com/apache/flink/pull/874#discussion_r33770473
--- Diff: flink-staging/flink-ml/src/main/scala/org/apache/flink/ml/classification/SVM.scala ---
@@ -187,6 +200,34 @@ class SVM extends Predictor[SVM] {
parameters.add(Seed, seed)
this
}
+
+ /** Sets the threshold above which elements are classified as positive.
+ *
+ * The [[predict ]]and [[evaluate]]functions will return +1.0 for items with a decision function
+ * value above this threshold, and -1.0 for items below it.
+ * @param threshold
+ * @return
+ */
+ def setThreshold(threshold: Double): SVM = {
+ parameters.add(ThresholdValue, threshold)
+ this
+ }
+
+ /** Sets whether the predictions should return the raw decision function value or the
+ * thresholded binary value.
+ *
+ * When setting this to true, predict and evaluate return the raw decision value, which is
+ * the distance from the separating hyperplane.
+ * When setting this to false, they return thresholded (+1.0, -1.0) values.
+ *
+ * @param outputDecisionFunction When set to true, [[predict ]]and [[evaluate]] return the raw
--- End diff --
Will fix
---
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.
---
[GitHub] flink pull request: [FLINK-2297] [ml] Add threshold setting for SV...
Posted by tillrohrmann <gi...@git.apache.org>.
Github user tillrohrmann commented on the pull request:
https://github.com/apache/flink/pull/874#issuecomment-117546335
Looks good the PR. I had only some minor comments.
---
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.
---
[GitHub] flink pull request: [FLINK-2297] [ml] Add threshold setting for SV...
Posted by thvasilo <gi...@git.apache.org>.
Github user thvasilo commented on a diff in the pull request:
https://github.com/apache/flink/pull/874#discussion_r33663520
--- Diff: flink-staging/flink-ml/src/main/scala/org/apache/flink/ml/classification/SVM.scala ---
@@ -242,8 +275,21 @@ object SVM{
}
}
- override def predict(value: T, model: DenseVector): Double = {
- value.asBreeze dot model.asBreeze
+ override def predict(value: T, model: DenseVector, predictParameters: ParameterMap):
+ Double = {
+ val thresholdOption = predictParameters.get(Threshold)
+
+ val rawValue = value.asBreeze dot model.asBreeze
+ // If the Threshold option has been reset, we will get back a Some(None) thresholdOption
+ // causing the exception when we try to get the value. In that case we just return the
+ // raw value
+ try {
+ val thresOptionValue = thresholdOption.get
+ if (rawValue > thresOptionValue) 1.0 else -1.0
+ }
+ catch {
+ case e: java.lang.ClassCastException => rawValue
+ }
--- End diff --
This relates to the previous discussion:
I do believe we want this turned on by default, when you train a binary classifier you expect that `predict` will return binary labels, not the decision function values.
So if we have `None` as default, the user could write:
```scala
val svm = SVM().
setBlocks(env.getParallelism)
svm.fit(train)
val eval = svm.evaluate(test)
```
and the eval output would not make sense, but if he wrote
```scala
val svm = SVM().
setBlocks(env.getParallelism).
setThreshold(0.0)
svm.fit(train)
val eval = svm.evaluate(test)
```
it would.
---
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.
---
[GitHub] flink pull request: [FLINK-2297] [ml] Add threshold setting for SV...
Posted by thvasilo <gi...@git.apache.org>.
Github user thvasilo commented on a diff in the pull request:
https://github.com/apache/flink/pull/874#discussion_r33659872
--- Diff: docs/libs/ml/svm.md ---
@@ -186,5 +199,5 @@ svm.fit(trainingDS)
val testingDS: DataSet[Vector] = env.readVectorFile(pathToTestingFile)
// Calculate the predictions for the testing data set
-val predictionDS: DataSet[LabeledVector] = svm.predict(testingDS)
+val predictionDS: DataSet[(FlinkVector, Double)] = svm.predict(testingDS)
--- End diff --
Maybe if I add a comment clarifying what I'm doing? My problems is that Scala Vector is imported by default and if you don't import the FlinkML Vector it will lead to problems.
---
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.
---
[GitHub] flink pull request: [FLINK-2297] [ml] Add threshold setting for SV...
Posted by tillrohrmann <gi...@git.apache.org>.
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/874#discussion_r33659300
--- Diff: flink-staging/flink-ml/src/main/scala/org/apache/flink/ml/pipeline/Predictor.scala ---
@@ -121,7 +121,7 @@ object Predictor {
input.mapWithBcVariable(model){
(element, model) => {
- (element, predictOperation.predict(element, model))
+ (element, predictOperation.predict(element, model, resultingParameters))
--- End diff --
Do we want to give the parameters to the `PredictOperation`? You could extract all necessary information in the `getModel` method of the `PredictOperation`.
---
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.
---
[GitHub] flink pull request: [FLINK-2297] [ml] Add threshold setting for SV...
Posted by thvasilo <gi...@git.apache.org>.
Github user thvasilo commented on a diff in the pull request:
https://github.com/apache/flink/pull/874#discussion_r33661400
--- Diff: docs/libs/ml/svm.md ---
@@ -186,5 +199,5 @@ svm.fit(trainingDS)
val testingDS: DataSet[Vector] = env.readVectorFile(pathToTestingFile)
// Calculate the predictions for the testing data set
-val predictionDS: DataSet[LabeledVector] = svm.predict(testingDS)
+val predictionDS: DataSet[(FlinkVector, Double)] = svm.predict(testingDS)
--- End diff --
I prefer FlinkVector in the library code at least to make sure we differentiate from Scala and Breeze. But you are right, for the example this import should suffice.
---
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.
---
[GitHub] flink pull request: [FLINK-2297] [ml] Add threshold setting for SV...
Posted by tillrohrmann <gi...@git.apache.org>.
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/874#discussion_r33658585
--- Diff: docs/libs/ml/svm.md ---
@@ -144,30 +145,42 @@ The SVM implementation can be controlled by the following parameters:
<td><strong>Stepsize</strong></td>
<td>
<p>
- Defines the initial step size for the updates of the weight vector.
- The larger the step size is, the larger will be the contribution of the weight vector updates to the next weight vector value.
+ Defines the initial step size for the updates of the weight vector.
+ The larger the step size is, the larger will be the contribution of the weight vector updates to the next weight vector value.
The effective scaling of the updates is $\frac{stepsize}{blocks}$.
- This value has to be tuned in case that the algorithm becomes unstable.
+ This value has to be tuned in case that the algorithm becomes unstable.
(Default value: <strong>1.0</strong>)
</p>
</td>
</tr>
<tr>
- <td><strong>Seed</strong></td>
+ <td><strong>Threshold</strong></td>
<td>
<p>
- Defines the seed to initialize the random number generator.
- The seed directly controls which data points are chosen for the SDCA method.
- (Default value: <strong>0</strong>)
+ Defines the limiting value for the decision function above which examples are labeled as
+ positive (+1.0). Examples with a decision function value below this value are classified
+ as negative (-1.0). In order to get the raw decision function value you need to
+ unset this parameter using the [[clearThreshold()]] function. (Default value: <strong>0.0</strong>)
--- End diff --
Do we want to have this turned on by default?
---
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.
---
[GitHub] flink pull request: [FLINK-2297] [ml] Add threshold setting for SV...
Posted by tillrohrmann <gi...@git.apache.org>.
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/874#discussion_r33659031
--- Diff: flink-staging/flink-ml/src/main/scala/org/apache/flink/ml/classification/SVM.scala ---
@@ -242,8 +275,21 @@ object SVM{
}
}
- override def predict(value: T, model: DenseVector): Double = {
- value.asBreeze dot model.asBreeze
+ override def predict(value: T, model: DenseVector, predictParameters: ParameterMap):
+ Double = {
+ val thresholdOption = predictParameters.get(Threshold)
+
+ val rawValue = value.asBreeze dot model.asBreeze
+ // If the Threshold option has been reset, we will get back a Some(None) thresholdOption
+ // causing the exception when we try to get the value. In that case we just return the
+ // raw value
+ try {
+ val thresOptionValue = thresholdOption.get
+ if (rawValue > thresOptionValue) 1.0 else -1.0
+ }
+ catch {
+ case e: java.lang.ClassCastException => rawValue
+ }
--- End diff --
I think it would be better if we have a `parameters.clear` or `parameters.remove` method which removes the value from the map.
---
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.
---
[GitHub] flink pull request: [FLINK-2297] [ml] Add threshold setting for SV...
Posted by thvasilo <gi...@git.apache.org>.
Github user thvasilo commented on a diff in the pull request:
https://github.com/apache/flink/pull/874#discussion_r33770712
--- Diff: flink-staging/flink-ml/src/main/scala/org/apache/flink/ml/classification/SVM.scala ---
@@ -232,7 +281,13 @@ object SVM{
implicit def predictVectors[T <: Vector] = {
--- End diff --
Will add.
---
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.
---
[GitHub] flink pull request: [FLINK-2297] [ml] Add threshold setting for SV...
Posted by tillrohrmann <gi...@git.apache.org>.
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/874#discussion_r33658953
--- Diff: flink-staging/flink-ml/src/main/scala/org/apache/flink/ml/classification/SVM.scala ---
@@ -187,6 +196,26 @@ class SVM extends Predictor[SVM] {
parameters.add(Seed, seed)
this
}
+
+ /** Sets the threshold above which elements are classified as positive
+ *
+ * @param threshold
+ * @return
+ */
+ def setThreshold(threshold: Double): SVM = {
+ parameters.add(Threshold, threshold)
+ this
+ }
+
+ /** Clears the classification threshold, predictions made after calling this function will have
+ * the raw decision function value.
+ *
+ * @return
+ */
+ def clearThreshold(): SVM = {
+ parameters.add(Threshold, Option.empty[Double])
--- End diff --
It would be better if we have a `parameters.clear(Threshold)` method 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.
---
[GitHub] flink pull request: [FLINK-2297] [ml] Add threshold setting for SV...
Posted by thvasilo <gi...@git.apache.org>.
Github user thvasilo commented on a diff in the pull request:
https://github.com/apache/flink/pull/874#discussion_r33772158
--- Diff: flink-staging/flink-ml/src/test/scala/org/apache/flink/ml/classification/SVMITSuite.scala ---
@@ -27,6 +28,8 @@ class SVMITSuite extends FlatSpec with Matchers with FlinkTestBase {
behavior of "The SVM using CoCoA implementation"
+ //TODO: Do SVM setup in a function
--- End diff --
Will remove
---
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.
---
[GitHub] flink pull request: [FLINK-2297] [ml] Add threshold setting for SV...
Posted by tillrohrmann <gi...@git.apache.org>.
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/874#discussion_r33765000
--- Diff: flink-staging/flink-ml/src/test/scala/org/apache/flink/ml/classification/SVMITSuite.scala ---
@@ -69,19 +70,38 @@ class SVMITSuite extends FlatSpec with Matchers with FlinkTestBase {
svm.fit(trainingDS)
- val threshold = 0.0
-
- val predictionPairs = svm.evaluate(test).map {
- truthPrediction =>
- val truth = truthPrediction._1
- val prediction = truthPrediction._2
- val thresholdedPrediction = if (prediction > threshold) 1.0 else -1.0
- (truth, thresholdedPrediction)
- }
+ val predictionPairs = svm.evaluate(test)
val absoluteErrorSum = predictionPairs.collect().map{
case (truth, prediction) => Math.abs(truth - prediction)}.sum
absoluteErrorSum should be < 15.0
}
+
+ it should "be possible to get the raw decision function values" in {
+ val env = ExecutionEnvironment.getExecutionEnvironment
+
+ val svm = SVM().
+ setBlocks(env.getParallelism).
+ setIterations(100).
+ setLocalIterations(100).
+ setRegularization(0.002).
+ setStepsize(0.1).
+ setSeed(0).
+ clearThreshold()
+
+ val trainingDS = env.fromCollection(Classification.trainingData)
+
+ val test = trainingDS.map(x => x.vector)
+
+ svm.fit(trainingDS)
+
+ val predictions: DataSet[(FlinkVector, Double)] = svm.predict(test)
+
+ val preds = predictions.map(vectorLabel => vectorLabel._2).collect()
+
+ preds.max should be > 1.0
--- End diff --
I think we should resolve this before merging.
---
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.
---
[GitHub] flink pull request: [FLINK-2297] [ml] Add threshold setting for SV...
Posted by thvasilo <gi...@git.apache.org>.
Github user thvasilo commented on a diff in the pull request:
https://github.com/apache/flink/pull/874#discussion_r33660212
--- Diff: flink-staging/flink-ml/src/main/scala/org/apache/flink/ml/common/ParameterMap.scala ---
@@ -112,7 +112,7 @@ object ParameterMap {
*
* @tparam T Type of parameter value associated to this parameter key
*/
-trait Parameter[T] {
+trait Parameter[+T] {
--- End diff --
This was initially needed to get the hack working, I will remove when I rework the ParameterMap
---
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.
---
[GitHub] flink pull request: [FLINK-2297] [ml] Add threshold setting for SV...
Posted by thvasilo <gi...@git.apache.org>.
Github user thvasilo commented on a diff in the pull request:
https://github.com/apache/flink/pull/874#discussion_r33659959
--- Diff: flink-staging/flink-ml/src/main/scala/org/apache/flink/ml/classification/SVM.scala ---
@@ -187,6 +196,26 @@ class SVM extends Predictor[SVM] {
parameters.add(Seed, seed)
this
}
+
+ /** Sets the threshold above which elements are classified as positive
+ *
+ * @param threshold
+ * @return
+ */
+ def setThreshold(threshold: Double): SVM = {
+ parameters.add(Threshold, threshold)
+ this
+ }
+
+ /** Clears the classification threshold, predictions made after calling this function will have
+ * the raw decision function value.
+ *
+ * @return
+ */
+ def clearThreshold(): SVM = {
+ parameters.add(Threshold, Option.empty[Double])
--- End diff --
>I'm surprised that this compiles
That's a good indication this is a hack then ;)
>It would be better if we have a parameters.clear(Threshold) method here.
Yes, I'll figure out a way to rework this by changing the ParameterMap.
---
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.
---
[GitHub] flink pull request: [FLINK-2297] [ml] Add threshold setting for SV...
Posted by tillrohrmann <gi...@git.apache.org>.
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/874#discussion_r33764958
--- Diff: flink-staging/flink-ml/src/test/scala/org/apache/flink/ml/classification/SVMITSuite.scala ---
@@ -27,6 +28,8 @@ class SVMITSuite extends FlatSpec with Matchers with FlinkTestBase {
behavior of "The SVM using CoCoA implementation"
+ //TODO: Do SVM setup in a function
--- End diff --
Why not resolving this TODO?
---
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.
---
[GitHub] flink pull request: [FLINK-2297] [ml] Add threshold setting for SV...
Posted by tillrohrmann <gi...@git.apache.org>.
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/874#discussion_r33658909
--- Diff: flink-staging/flink-ml/src/main/scala/org/apache/flink/ml/classification/SVM.scala ---
@@ -242,8 +275,21 @@ object SVM{
}
}
- override def predict(value: T, model: DenseVector): Double = {
- value.asBreeze dot model.asBreeze
+ override def predict(value: T, model: DenseVector, predictParameters: ParameterMap):
+ Double = {
+ val thresholdOption = predictParameters.get(Threshold)
+
+ val rawValue = value.asBreeze dot model.asBreeze
+ // If the Threshold option has been reset, we will get back a Some(None) thresholdOption
+ // causing the exception when we try to get the value. In that case we just return the
+ // raw value
+ try {
+ val thresOptionValue = thresholdOption.get
+ if (rawValue > thresOptionValue) 1.0 else -1.0
+ }
+ catch {
+ case e: java.lang.ClassCastException => rawValue
+ }
--- End diff --
This is indeed hacky. I would vote for reworking it. The problem is that you're assigning an `Option` to the parameter map where a `double` is required. I'm actually surprised that this compiles.
---
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.
---
[GitHub] flink pull request: [FLINK-2297] [ml] Add threshold setting for SV...
Posted by thvasilo <gi...@git.apache.org>.
Github user thvasilo commented on a diff in the pull request:
https://github.com/apache/flink/pull/874#discussion_r33662458
--- Diff: flink-staging/flink-ml/src/main/scala/org/apache/flink/ml/pipeline/Predictor.scala ---
@@ -121,7 +121,7 @@ object Predictor {
input.mapWithBcVariable(model){
(element, model) => {
- (element, predictOperation.predict(element, model))
+ (element, predictOperation.predict(element, model, resultingParameters))
--- End diff --
In this case the output of `predict` depends on the parameter settings. With access only to `value` and `model` we can't know what to output otherwise.
---
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.
---
[GitHub] flink pull request: [FLINK-2297] [ml] Add threshold setting for SV...
Posted by tillrohrmann <gi...@git.apache.org>.
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/874#discussion_r33660985
--- Diff: docs/libs/ml/svm.md ---
@@ -186,5 +199,5 @@ svm.fit(trainingDS)
val testingDS: DataSet[Vector] = env.readVectorFile(pathToTestingFile)
// Calculate the predictions for the testing data set
-val predictionDS: DataSet[LabeledVector] = svm.predict(testingDS)
+val predictionDS: DataSet[(FlinkVector, Double)] = svm.predict(testingDS)
--- End diff --
Why not writing `import org.apache.flink.ml.math.Vector`?
---
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.
---
[GitHub] flink pull request: [FLINK-2297] [ml] Add threshold setting for SV...
Posted by tillrohrmann <gi...@git.apache.org>.
Github user tillrohrmann commented on the pull request:
https://github.com/apache/flink/pull/874#issuecomment-117993575
Some more minor comments.
---
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.
---
[GitHub] flink pull request: [FLINK-2297] [ml] Add threshold setting for SV...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/flink/pull/874
---
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.
---
[GitHub] flink pull request: [FLINK-2297] [ml] Add threshold setting for SV...
Posted by thvasilo <gi...@git.apache.org>.
Github user thvasilo commented on a diff in the pull request:
https://github.com/apache/flink/pull/874#discussion_r33770569
--- Diff: flink-staging/flink-ml/src/main/scala/org/apache/flink/ml/classification/SVM.scala ---
@@ -242,8 +297,17 @@ object SVM{
}
}
- override def predict(value: T, model: DenseVector): Double = {
- value.asBreeze dot model.asBreeze
+ override def predict(value: T, model: DenseVector):
+ Double = {
+
+ val rawValue = value.asBreeze dot model.asBreeze
+
+ if (outputDecisionFunction) {
+ rawValue
+ }
+ else {
--- End diff --
is this OK
```scala
override def predict(value: T, model: DenseVector): Double = {
val rawValue = value.asBreeze dot model.asBreeze
if (outputDecisionFunction) {
rawValue
}
else {
if (rawValue > thresholdValue) 1.0 else -1.0
}
}
```
---
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.
---
[GitHub] flink pull request: [FLINK-2297] [ml] Add threshold setting for SV...
Posted by tillrohrmann <gi...@git.apache.org>.
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/874#discussion_r33661424
--- Diff: flink-staging/flink-ml/src/main/scala/org/apache/flink/ml/pipeline/Predictor.scala ---
@@ -121,7 +121,7 @@ object Predictor {
input.mapWithBcVariable(model){
(element, model) => {
- (element, predictOperation.predict(element, model))
+ (element, predictOperation.predict(element, model, resultingParameters))
--- End diff --
Hmm I'm not so sure either what is the best way to go here. My initial idea was to isolate the predict operation as much as possible from the code necessary to do the setup. I don't like the `ParameterMap` much, because it allows you to have access to much more information than necessary. IMO it's better to extract the information once at one place and then have an explicit representation of the information.
---
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.
---
[GitHub] flink pull request: [FLINK-2297] [ml] Add threshold setting for SV...
Posted by tillrohrmann <gi...@git.apache.org>.
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/874#discussion_r33771778
--- Diff: flink-staging/flink-ml/src/test/scala/org/apache/flink/ml/classification/SVMITSuite.scala ---
@@ -27,6 +28,8 @@ class SVMITSuite extends FlatSpec with Matchers with FlinkTestBase {
behavior of "The SVM using CoCoA implementation"
+ //TODO: Do SVM setup in a function
--- End diff --
Can't we copy the part to this PR? Merging open TODOs is not so nice.
---
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.
---
[GitHub] flink pull request: [FLINK-2297] [ml] Add threshold setting for SV...
Posted by tillrohrmann <gi...@git.apache.org>.
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/874#discussion_r33771828
--- Diff: flink-staging/flink-ml/src/main/scala/org/apache/flink/ml/classification/SVM.scala ---
@@ -242,8 +297,17 @@ object SVM{
}
}
- override def predict(value: T, model: DenseVector): Double = {
- value.asBreeze dot model.asBreeze
+ override def predict(value: T, model: DenseVector):
+ Double = {
+
+ val rawValue = value.asBreeze dot model.asBreeze
+
+ if (outputDecisionFunction) {
+ rawValue
+ }
+ else {
--- End diff --
Actually I meant the else statement.
'''
if (outputDecisionFunction) {
rawValue
} else {
...
}
'''
---
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.
---
[GitHub] flink pull request: [FLINK-2297] [ml] Add threshold setting for SV...
Posted by tillrohrmann <gi...@git.apache.org>.
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/874#discussion_r33659114
--- Diff: flink-staging/flink-ml/src/main/scala/org/apache/flink/ml/common/ParameterMap.scala ---
@@ -112,7 +112,7 @@ object ParameterMap {
*
* @tparam T Type of parameter value associated to this parameter key
*/
-trait Parameter[T] {
+trait Parameter[+T] {
--- End diff --
For what purposes do we need a covariant parameter type?
---
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.
---
[GitHub] flink pull request: [FLINK-2297] [ml] Add threshold setting for SV...
Posted by tillrohrmann <gi...@git.apache.org>.
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/874#discussion_r33658638
--- Diff: docs/libs/ml/svm.md ---
@@ -186,5 +199,5 @@ svm.fit(trainingDS)
val testingDS: DataSet[Vector] = env.readVectorFile(pathToTestingFile)
// Calculate the predictions for the testing data set
-val predictionDS: DataSet[LabeledVector] = svm.predict(testingDS)
+val predictionDS: DataSet[(FlinkVector, Double)] = svm.predict(testingDS)
--- End diff --
Maybe we shouldn't use renaming of imports here. This might be confusing for people not so familiar with Scala.
---
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.
---
[GitHub] flink pull request: [FLINK-2297] [ml] Add threshold setting for SV...
Posted by tillrohrmann <gi...@git.apache.org>.
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/874#discussion_r33658906
--- Diff: flink-staging/flink-ml/src/main/scala/org/apache/flink/ml/classification/SVM.scala ---
@@ -187,6 +196,26 @@ class SVM extends Predictor[SVM] {
parameters.add(Seed, seed)
this
}
+
+ /** Sets the threshold above which elements are classified as positive
+ *
+ * @param threshold
+ * @return
+ */
+ def setThreshold(threshold: Double): SVM = {
+ parameters.add(Threshold, threshold)
+ this
+ }
+
+ /** Clears the classification threshold, predictions made after calling this function will have
+ * the raw decision function value.
+ *
+ * @return
+ */
+ def clearThreshold(): SVM = {
+ parameters.add(Threshold, Option.empty[Double])
--- End diff --
I'm surprised that this compiles because you should only be able to add a `Double` 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.
---
[GitHub] flink pull request: [FLINK-2297] [ml] Add threshold setting for SV...
Posted by tillrohrmann <gi...@git.apache.org>.
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/874#discussion_r33765503
--- Diff: flink-staging/flink-ml/src/main/scala/org/apache/flink/ml/classification/SVM.scala ---
@@ -232,7 +281,13 @@ object SVM{
implicit def predictVectors[T <: Vector] = {
--- End diff --
Can we add some scala docs? It's actually my fault because it was already missing when I merged the change of the predict operation.
---
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.
---
[GitHub] flink pull request: [FLINK-2297] [ml] Add threshold setting for SV...
Posted by tillrohrmann <gi...@git.apache.org>.
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/874#discussion_r33662137
--- Diff: flink-staging/flink-ml/src/main/scala/org/apache/flink/ml/classification/SVM.scala ---
@@ -242,8 +275,21 @@ object SVM{
}
}
- override def predict(value: T, model: DenseVector): Double = {
- value.asBreeze dot model.asBreeze
+ override def predict(value: T, model: DenseVector, predictParameters: ParameterMap):
+ Double = {
+ val thresholdOption = predictParameters.get(Threshold)
+
+ val rawValue = value.asBreeze dot model.asBreeze
+ // If the Threshold option has been reset, we will get back a Some(None) thresholdOption
+ // causing the exception when we try to get the value. In that case we just return the
+ // raw value
+ try {
+ val thresOptionValue = thresholdOption.get
+ if (rawValue > thresOptionValue) 1.0 else -1.0
+ }
+ catch {
+ case e: java.lang.ClassCastException => rawValue
+ }
--- End diff --
Do we want to have a different default value thane `None`? IMO it makes more sense for the user to define the threshold than to disable it. This makes more explicit to him that he has reason about the threshold value.
---
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.
---
[GitHub] flink pull request: [FLINK-2297] [ml] Add threshold setting for SV...
Posted by tillrohrmann <gi...@git.apache.org>.
Github user tillrohrmann commented on the pull request:
https://github.com/apache/flink/pull/874#issuecomment-118013718
LGTM. Will merge it with the next batch.
---
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.
---
[GitHub] flink pull request: [FLINK-2297] [ml] Add threshold setting for SV...
Posted by thvasilo <gi...@git.apache.org>.
Github user thvasilo commented on a diff in the pull request:
https://github.com/apache/flink/pull/874#discussion_r33660746
--- Diff: flink-staging/flink-ml/src/main/scala/org/apache/flink/ml/pipeline/Predictor.scala ---
@@ -121,7 +121,7 @@ object Predictor {
input.mapWithBcVariable(model){
(element, model) => {
- (element, predictOperation.predict(element, model))
+ (element, predictOperation.predict(element, model, resultingParameters))
--- End diff --
I could have a `predictParameters` variable in the SVM `PredictOperation` and assign it a value in `getModel` then.
Does that sound better?
---
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.
---
[GitHub] flink pull request: [FLINK-2297] [ml] Add threshold setting for SV...
Posted by thvasilo <gi...@git.apache.org>.
Github user thvasilo commented on a diff in the pull request:
https://github.com/apache/flink/pull/874#discussion_r33659736
--- Diff: docs/libs/ml/svm.md ---
@@ -144,30 +145,42 @@ The SVM implementation can be controlled by the following parameters:
<td><strong>Stepsize</strong></td>
<td>
<p>
- Defines the initial step size for the updates of the weight vector.
- The larger the step size is, the larger will be the contribution of the weight vector updates to the next weight vector value.
+ Defines the initial step size for the updates of the weight vector.
+ The larger the step size is, the larger will be the contribution of the weight vector updates to the next weight vector value.
The effective scaling of the updates is $\frac{stepsize}{blocks}$.
- This value has to be tuned in case that the algorithm becomes unstable.
+ This value has to be tuned in case that the algorithm becomes unstable.
(Default value: <strong>1.0</strong>)
</p>
</td>
</tr>
<tr>
- <td><strong>Seed</strong></td>
+ <td><strong>Threshold</strong></td>
<td>
<p>
- Defines the seed to initialize the random number generator.
- The seed directly controls which data points are chosen for the SDCA method.
- (Default value: <strong>0</strong>)
+ Defines the limiting value for the decision function above which examples are labeled as
+ positive (+1.0). Examples with a decision function value below this value are classified
+ as negative (-1.0). In order to get the raw decision function value you need to
+ unset this parameter using the [[clearThreshold()]] function. (Default value: <strong>0.0</strong>)
--- End diff --
No we don't that's actually a bug I had forgotten about. I'll fix it 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.
---
[GitHub] flink pull request: [FLINK-2297] [ml] Add threshold setting for SV...
Posted by thvasilo <gi...@git.apache.org>.
Github user thvasilo commented on a diff in the pull request:
https://github.com/apache/flink/pull/874#discussion_r33570815
--- Diff: flink-staging/flink-ml/src/main/scala/org/apache/flink/ml/classification/SVM.scala ---
@@ -242,8 +275,21 @@ object SVM{
}
}
- override def predict(value: T, model: DenseVector): Double = {
- value.asBreeze dot model.asBreeze
+ override def predict(value: T, model: DenseVector, predictParameters: ParameterMap):
+ Double = {
+ val thresholdOption = predictParameters.get(Threshold)
+
+ val rawValue = value.asBreeze dot model.asBreeze
+ // If the Threshold option has been reset, we will get back a Some(None) thresholdOption
+ // causing the exception when we try to get the value. In that case we just return the
+ // raw value
+ try {
--- End diff --
This part is a bit hacky and will have a performance impact. However it allows to do what we want without needing to change the way ParameterMap works, and with the default behaviour being that the thresholdOption is set, the exception should be triggered most of the time.
---
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.
---
[GitHub] flink pull request: [FLINK-2297] [ml] Add threshold setting for SV...
Posted by thvasilo <gi...@git.apache.org>.
Github user thvasilo commented on a diff in the pull request:
https://github.com/apache/flink/pull/874#discussion_r33570883
--- Diff: flink-staging/flink-ml/src/test/scala/org/apache/flink/ml/classification/SVMITSuite.scala ---
@@ -69,19 +70,38 @@ class SVMITSuite extends FlatSpec with Matchers with FlinkTestBase {
svm.fit(trainingDS)
- val threshold = 0.0
-
- val predictionPairs = svm.evaluate(test).map {
- truthPrediction =>
- val truth = truthPrediction._1
- val prediction = truthPrediction._2
- val thresholdedPrediction = if (prediction > threshold) 1.0 else -1.0
- (truth, thresholdedPrediction)
- }
+ val predictionPairs = svm.evaluate(test)
val absoluteErrorSum = predictionPairs.collect().map{
case (truth, prediction) => Math.abs(truth - prediction)}.sum
absoluteErrorSum should be < 15.0
}
+
+ it should "be possible to get the raw decision function values" in {
+ val env = ExecutionEnvironment.getExecutionEnvironment
+
+ val svm = SVM().
+ setBlocks(env.getParallelism).
+ setIterations(100).
+ setLocalIterations(100).
+ setRegularization(0.002).
+ setStepsize(0.1).
+ setSeed(0).
+ clearThreshold()
+
+ val trainingDS = env.fromCollection(Classification.trainingData)
+
+ val test = trainingDS.map(x => x.vector)
+
+ svm.fit(trainingDS)
+
+ val predictions: DataSet[(FlinkVector, Double)] = svm.predict(test)
+
+ val preds = predictions.map(vectorLabel => vectorLabel._2).collect()
+
+ preds.max should be > 1.0
--- End diff --
Better way to check raw values vs. 1/-1
---
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.
---
[GitHub] flink pull request: [FLINK-2297] [ml] Add threshold setting for SV...
Posted by thvasilo <gi...@git.apache.org>.
Github user thvasilo commented on a diff in the pull request:
https://github.com/apache/flink/pull/874#discussion_r33770184
--- Diff: flink-staging/flink-ml/src/test/scala/org/apache/flink/ml/classification/SVMITSuite.scala ---
@@ -27,6 +28,8 @@ class SVMITSuite extends FlatSpec with Matchers with FlinkTestBase {
behavior of "The SVM using CoCoA implementation"
+ //TODO: Do SVM setup in a function
--- End diff --
It's done in the evaluation branch
---
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.
---
[GitHub] flink pull request: [FLINK-2297] [ml] Add threshold setting for SV...
Posted by tillrohrmann <gi...@git.apache.org>.
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/874#discussion_r33659515
--- Diff: flink-staging/flink-ml/src/test/scala/org/apache/flink/ml/classification/SVMITSuite.scala ---
@@ -69,19 +70,38 @@ class SVMITSuite extends FlatSpec with Matchers with FlinkTestBase {
svm.fit(trainingDS)
- val threshold = 0.0
-
- val predictionPairs = svm.evaluate(test).map {
- truthPrediction =>
- val truth = truthPrediction._1
- val prediction = truthPrediction._2
- val thresholdedPrediction = if (prediction > threshold) 1.0 else -1.0
- (truth, thresholdedPrediction)
- }
+ val predictionPairs = svm.evaluate(test)
val absoluteErrorSum = predictionPairs.collect().map{
case (truth, prediction) => Math.abs(truth - prediction)}.sum
absoluteErrorSum should be < 15.0
}
+
+ it should "be possible to get the raw decision function values" in {
+ val env = ExecutionEnvironment.getExecutionEnvironment
+
+ val svm = SVM().
+ setBlocks(env.getParallelism).
+ setIterations(100).
+ setLocalIterations(100).
+ setRegularization(0.002).
+ setStepsize(0.1).
+ setSeed(0).
+ clearThreshold()
+
+ val trainingDS = env.fromCollection(Classification.trainingData)
+
+ val test = trainingDS.map(x => x.vector)
+
+ svm.fit(trainingDS)
+
+ val predictions: DataSet[(FlinkVector, Double)] = svm.predict(test)
+
+ val preds = predictions.map(vectorLabel => vectorLabel._2).collect()
+
+ preds.max should be > 1.0
--- End diff --
hmm you could manually set the weight vector for which you are sure that something different than 1/-1 is calculated
---
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.
---
[GitHub] flink pull request: [FLINK-2297] [ml] Add threshold setting for SV...
Posted by thvasilo <gi...@git.apache.org>.
Github user thvasilo commented on a diff in the pull request:
https://github.com/apache/flink/pull/874#discussion_r33661657
--- Diff: flink-staging/flink-ml/src/main/scala/org/apache/flink/ml/classification/SVM.scala ---
@@ -242,8 +275,21 @@ object SVM{
}
}
- override def predict(value: T, model: DenseVector): Double = {
- value.asBreeze dot model.asBreeze
+ override def predict(value: T, model: DenseVector, predictParameters: ParameterMap):
+ Double = {
+ val thresholdOption = predictParameters.get(Threshold)
+
+ val rawValue = value.asBreeze dot model.asBreeze
+ // If the Threshold option has been reset, we will get back a Some(None) thresholdOption
+ // causing the exception when we try to get the value. In that case we just return the
+ // raw value
+ try {
+ val thresOptionValue = thresholdOption.get
+ if (rawValue > thresOptionValue) 1.0 else -1.0
+ }
+ catch {
+ case e: java.lang.ClassCastException => rawValue
+ }
--- End diff --
The problems is with how `get` is defined, if a value is not defined in the map, it will return the default value. So even if I clear a parameter, if its default value is not `None` (which is what we want here) `get` will return the default value.
---
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.
---
[GitHub] flink pull request: [FLINK-2297] [ml] Add threshold setting for SV...
Posted by thvasilo <gi...@git.apache.org>.
Github user thvasilo commented on a diff in the pull request:
https://github.com/apache/flink/pull/874#discussion_r33770251
--- Diff: flink-staging/flink-ml/src/test/scala/org/apache/flink/ml/classification/SVMITSuite.scala ---
@@ -69,19 +70,38 @@ class SVMITSuite extends FlatSpec with Matchers with FlinkTestBase {
svm.fit(trainingDS)
- val threshold = 0.0
-
- val predictionPairs = svm.evaluate(test).map {
- truthPrediction =>
- val truth = truthPrediction._1
- val prediction = truthPrediction._2
- val thresholdedPrediction = if (prediction > threshold) 1.0 else -1.0
- (truth, thresholdedPrediction)
- }
+ val predictionPairs = svm.evaluate(test)
val absoluteErrorSum = predictionPairs.collect().map{
case (truth, prediction) => Math.abs(truth - prediction)}.sum
absoluteErrorSum should be < 15.0
}
+
+ it should "be possible to get the raw decision function values" in {
+ val env = ExecutionEnvironment.getExecutionEnvironment
+
+ val svm = SVM().
+ setBlocks(env.getParallelism).
+ setIterations(100).
+ setLocalIterations(100).
+ setRegularization(0.002).
+ setStepsize(0.1).
+ setSeed(0).
+ clearThreshold()
+
+ val trainingDS = env.fromCollection(Classification.trainingData)
+
+ val test = trainingDS.map(x => x.vector)
+
+ svm.fit(trainingDS)
+
+ val predictions: DataSet[(FlinkVector, Double)] = svm.predict(test)
+
+ val preds = predictions.map(vectorLabel => vectorLabel._2).collect()
+
+ preds.max should be > 1.0
--- End diff --
Will do.
---
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.
---
[GitHub] flink pull request: [FLINK-2297] [ml] Add threshold setting for SV...
Posted by thvasilo <gi...@git.apache.org>.
Github user thvasilo commented on the pull request:
https://github.com/apache/flink/pull/874#issuecomment-117574076
After discussing with, we decided that one parameter determining the value of the threshold _and_ whether we output the thresholded values or not is ambiguous.
What we will do instead is provide another parameter that determines whether the prediction output will be thresholded or not, and the Threshold parameter only does one thing, determine the value of that threshold.
---
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.
---
[GitHub] flink pull request: [FLINK-2297] [ml] Add threshold setting for SV...
Posted by tillrohrmann <gi...@git.apache.org>.
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/874#discussion_r33764905
--- Diff: flink-staging/flink-ml/src/main/scala/org/apache/flink/ml/classification/SVM.scala ---
@@ -242,8 +297,17 @@ object SVM{
}
}
- override def predict(value: T, model: DenseVector): Double = {
- value.asBreeze dot model.asBreeze
+ override def predict(value: T, model: DenseVector):
+ Double = {
+
+ val rawValue = value.asBreeze dot model.asBreeze
+
+ if (outputDecisionFunction) {
+ rawValue
+ }
+ else {
--- End diff --
formatting
---
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.
---
[GitHub] flink pull request: [FLINK-2297] [ml] Add threshold setting for SV...
Posted by tillrohrmann <gi...@git.apache.org>.
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/874#discussion_r33764869
--- Diff: flink-staging/flink-ml/src/main/scala/org/apache/flink/ml/classification/SVM.scala ---
@@ -187,6 +200,34 @@ class SVM extends Predictor[SVM] {
parameters.add(Seed, seed)
this
}
+
+ /** Sets the threshold above which elements are classified as positive.
+ *
+ * The [[predict ]]and [[evaluate]]functions will return +1.0 for items with a decision function
+ * value above this threshold, and -1.0 for items below it.
+ * @param threshold
+ * @return
+ */
+ def setThreshold(threshold: Double): SVM = {
+ parameters.add(ThresholdValue, threshold)
+ this
+ }
+
+ /** Sets whether the predictions should return the raw decision function value or the
+ * thresholded binary value.
+ *
+ * When setting this to true, predict and evaluate return the raw decision value, which is
+ * the distance from the separating hyperplane.
+ * When setting this to false, they return thresholded (+1.0, -1.0) values.
+ *
+ * @param outputDecisionFunction When set to true, [[predict ]]and [[evaluate]] return the raw
--- End diff --
whitespace between [[predict]] and "and"
---
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.
---