You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by tillrohrmann <gi...@git.apache.org> on 2015/06/03 17:03:32 UTC

[GitHub] flink pull request: [FLINK-2116] [ml] Reusing predict operation fo...

GitHub user tillrohrmann opened a pull request:

    https://github.com/apache/flink/pull/772

    [FLINK-2116] [ml] Reusing predict operation for evaluation

    This PR adds an `evaluate` method to `Predictor` which takes a `DataSet[Testing]` and returns a `DataSet[(LabelType, LabelType)]`, where the first tuple field is the true label and the second field denotes the predicted label. The evaluation logic is defined via a `EvaluateDataSetOperation`.
    
    Since predicting test data and evaluate test data both use the same prediction logic, a new level  of abstraction was introduced. The old `PredictOperation` is now called `PredictDataSetOperation` and a new `PredictOperation` was defined. The `PredictOperation` takes an element of the dataset as well as the model of the associated `Predictor` and calculates one prediction.
    
    If one wants to implement the predict operation of a `Predictor` then one can do it on the level of `PredictDataSetOperation` which gives you access to the `DataSet` of input elements or on the level of `PredictOperation`. If one chooses the latter, then the system will automatically apply this operation to all elements of the input `DataSet` (see `Predictor.defaultPredictDataSetOperation`).
    
    Having defined a `PredictOperation` allows to automatically call `evaluate` for this `Predictor` without having to define a `EvaluateDataSetOperation`. The only constraint is that the input data has to be `DataSet[(TestingType, LabelType)]`. The input is thus a tuple with a testing value and the true label value. The system will then calculate the prediction for the testing value and return a `DataSet[(LabelType, LabelType)]` where the first field value of the tuple is the true label value and the second field value is the predicted label value.
    
    What do you think of these changes? Will they ease the development of future `Predictor`s?

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

    $ git pull https://github.com/tillrohrmann/flink evaluatePredictor

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

    https://github.com/apache/flink/pull/772.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 #772
    
----
commit 49c02514a6a23d7ef95ce46966ff7ee7a1f407ad
Author: Till Rohrmann <tr...@apache.org>
Date:   2015-06-02T12:34:27Z

    [FLINK-2116] [ml] Adds evaluate method to Predictor. Adds PredictOperation which can be reused by evaluate if the input data is of the format (TestingType, LabelType) where the second tuple field represents the true label.

----


---
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-2116] [ml] Reusing predict operation fo...

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

    https://github.com/apache/flink/pull/772#issuecomment-114117337
  
    OK so as I understand it this PR is about splitting the prediction with Vector and LabeledVector input into two functions instead of one as it is now.
    
    This allows us to avoid some code duplication and have better defined semantics. I'm fine with that.
    
    I would change the name from *Evaluate* and *evaluation* to something different though. Evaluation for me means making predictions *and* evaluating the result using some aggregating metric. The result of an evaluation should be a score (real number).
    
    Something like labeledPrediction or predictLabeled makes more sense. Any other suggestions?


---
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-2116] [ml] Reusing predict operation fo...

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

    https://github.com/apache/flink/pull/772#issuecomment-109926023
  
    I created a [JIRA ticket](https://issues.apache.org/jira/browse/FLINK-2157?filter=12331885) for the evaluation of predictors. I think we should deal with it separately in order to keep the PR small and reviewable. But I agree, this is an important feature for any ML library.


---
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-2116] [ml] Reusing predict operation fo...

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

    https://github.com/apache/flink/pull/772#issuecomment-115198124
  
    Actually I wouldn't call it predictSomething, because then we're again quite close to the former problem that we have a method whose semantics depend on the provided type. And this only confuses users. 
    
    My concern is that the user does not really know what `predictLabeled` means. Apparently it is something similar to `predict` but with a label. But what is the label? Does it mean that I can apply `predict` on `T <: Vector` and `predictLabeled` on `LabeledVector`? Does it mean that I get a labeled result type? But don't I already get it with `predict`? Do I have to provide a type with a label or can I also supply a vector? 
    
    IMO, the prediction which also returns the true label value deserves a more distinguishable name than `predictSomething`, because it has different semantics. I can't think of something better than `evaluate` at the moment. But it makes it clear that the user has to provide some evaluation `DataSet`, meaning some labeled data.


---
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-2116] [ml] Reusing predict operation fo...

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

    https://github.com/apache/flink/pull/772#issuecomment-108482112
  
    Great. This is exactly what I had in mind. 
    There is perhaps another feature we could incorporate. Every algorithm has some performance measure to so it can be evaluated on a test data set. We could incorporate this as a parameter in the model. As soon as evaluate gets called, this parameter is set to the performance value. It could be squared-error for MLR, or F-score and accuracy, etc. for SVM, and so on. 
    User accesses this performance measure with a simple instance.get and (most likely) prints it, so we don't need to make it of the same type across different algorithms. Every Predictor can have its own performance object.



---
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-2116] [ml] Reusing predict operation fo...

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

    https://github.com/apache/flink/pull/772


---
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-2116] [ml] Reusing predict operation fo...

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

    https://github.com/apache/flink/pull/772#issuecomment-115660064
  
    So I guess we are keeping the naming for now. If we come up with something to avoid confusion with Scorer.evaluate() in the future we can take another look at this.


---
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-2116] [ml] Reusing predict operation fo...

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

    https://github.com/apache/flink/pull/772#issuecomment-115697593
  
    Yeah, it's not carved into stone. I mean this holds true for the whole library.


---
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.
---