Move evaluation results to separate classes #1663

robosek · 2018-11-17T21:07:23Z

Fixes #1280

NOTE:

Moved results from (Ranking, Multiclass, Regression, Clustering, Binary) evaluators to separte classes under Evaluators/Metrics directory.
Adjusted types in dependent projects classes.
Build Release and Debug successful on local 👍

…nsistency

Zruty0

Thanks @robosek for your contribution!

It looks good to me, except could you please move all these new classes to Microsoft.ML.Data ?

robosek · 2018-11-19T09:58:04Z

@Zruty0 You mean I should change only the namespaces to Microsoft.ML.Data of new classes right? #Resolved

Zruty0 · 2018-11-19T18:33:22Z

@robosek yes, that's what I meant #Resolved

sfilipi · 2018-11-20T06:13:08Z

src/Microsoft.ML.Data/Evaluators/Metrics/ClusteringMetrics.cs

+        public double Nmi { get; }
+
+        /// <summary>
+        /// Average Score. For the K-Means algorithm, the 'score' is the distance from the centroid to the example.


' [](start = 58, length = 1)

The apostrophes were already there, but we might need to XML encode them, here and below. #Resolved

sfilipi · 2018-11-20T06:15:07Z

test/Microsoft.ML.Tests/ScenariosWithDirectInstantiation/SentimentPredictionTests.cs

            var metricsDic = evaluator.Evaluate(dataEval);

-            return BinaryClassificationMetrics.FromMetrics(env, metricsDic["OverallMetrics"], metricsDic["ConfusionMatrix"])[0];
+            return Microsoft.ML.Legacy.Models.BinaryClassificationMetrics


Microsoft.ML.Legacy.Models [](start = 19, length = 26)

does this need to be Microsoft.ML.Data?

Actually before my change it was BinaryClassificationMetrics from Microsoft.ML.Legacy.Models but I had to add full name of class to distinguish between new BinaryClassificationMetrics from Micorosft.ML.Data and the legacy one.

sfilipi · 2018-11-20T06:15:28Z

test/Microsoft.ML.Tests/ScenariosWithDirectInstantiation/SentimentPredictionTests.cs

        }

-        private BinaryClassificationMetrics EvaluateBinary(IHostEnvironment env, IDataView scoredData)
+        private Microsoft.ML.Legacy.Models.BinaryClassificationMetrics EvaluateBinary(IHostEnvironment env, IDataView scoredData)


Microsoft.ML.Legacy.Models [](start = 16, length = 26)

Microsoft.ML.Data?

The same as above

Ivanidzo4ka · 2018-11-27T21:37:25Z

src/Microsoft.ML.Data/Evaluators/Metrics/ClusteringMetrics.cs

+        public double AvgMinScore { get; }
+
+        /// <summary>
+        /// <a href="https://en.wikipedia.org/wiki/Davies�Bouldin_index">Davies-Bouldin Index</a>


� [](start = 57, length = 1)

something went wrong with this hypen. I can't open link.
[https://en.wikipedia.org/wiki/Davies–Bouldin_index](https://en.wikipedia.org/wiki/Davies–Bouldin_index`) works fine

Ivanidzo4ka · 2018-11-27T21:47:05Z

src/Microsoft.ML.Data/Evaluators/Metrics/RankerMetrics.cs

+    {
+        /// <summary>
+        /// Normalized Discounted Cumulative Gain
+        /// <a href="https://github.com/dotnet/machinelearning/tree/master/docs/images/ndcg.png"></a>


ndcg [](start = 87, length = 4)

need to be NDCG.png

Ivanidzo4ka · 2018-11-27T21:47:29Z

src/Microsoft.ML.Data/Evaluators/Metrics/RankerMetrics.cs

+        /// <a href="https://en.wikipedia.org/wiki/Discounted_cumulative_gain">Discounted Cumulative gain</a>
+        /// is the sum of the gains, for all the instances i, normalized by the natural logarithm of the instance + 1.
+        /// Note that unline the Wikipedia article, ML.Net uses the natural logarithm.
+        /// <a href="https://github.com/dotnet/machinelearning/tree/master/docs/images/dcg.png"></a>


dcg.png [](start = 87, length = 7)

need to be DCG.png

Ivanidzo4ka · 2018-11-27T21:52:17Z

src/Microsoft.ML.Data/Evaluators/Metrics/RankerMetrics.cs

+    public sealed class RankerMetrics
+    {
+        /// <summary>
+        /// Normalized Discounted Cumulative Gain


Normalized Discounted Cumulative Gain [](start = 12, length = 37)

Array of normalized discounted cumulative gains where i-th element represent NDCG@i (you can also phrase it as NDCG at i). #Closed

Ivanidzo4ka · 2018-11-27T21:52:39Z

src/Microsoft.ML.Data/Evaluators/Metrics/RankerMetrics.cs

+        /// </summary>
+        public double[] Ndcg { get; }
+
+        /// <summary>


Array of discounted cumulative gains where i-th element represent DCG@i (you can also phrase it as DCG at i). #Closed

Ivanidzo4ka · 2018-11-27T21:56:51Z

src/Microsoft.ML.Data/Evaluators/Metrics/RegressionMetrics.cs

+        public double Rms { get; }
+
+        /// <summary>
+        /// Gets the user defined loss function.


Gets the user [](start = 12, length = 13)

Gets the result of user defined loss function.
or
Get result of the user defined loss function.
English is hard.
My point is we don't return loss function we return result of it. #Closed

Ivanidzo4ka · 2018-11-27T21:58:17Z

src/Microsoft.ML.Data/Training/TrainingStaticExtensions.cs

 using System.Collections.Generic;
 using System.Linq;
 using System.Text;
+using Microsoft.ML.Data;


would be nice to sort them.

Ivanidzo4ka

Ivanidzo4ka · 2018-11-27T22:05:13Z

@robosek thank you for your contribution!
I left few comments, and you need to resolve some merge conflicts (it's end of cycle time, so people tend to push lot of PRs)
#Resolved

robosek · 2018-11-28T18:57:36Z

@Ivanidzo4ka thank you for your review. Descriptions are updated and conflicts are resolved (at least for a while 😃) #Resolved

Ivanidzo4ka · 2018-11-28T19:08:46Z

@robosek can you do following:
open command line and go to your machinelearning repository.
git submodule update --remote --merge
you should get following message in the end:

Submodule path 'src/Native/MatrixFactorizationNative/libmf': merged in '53a91e7e8c88463e97acfbbafb7134a6030860b3'

stage it and merge into your PR.

We have submodule which we recently updated in master, but in your branch it's still looking on old one, and for some reason git not smart enough to merge it properly.
Command above will point on latest master branch in libmf.

robosek · 2018-11-28T19:50:05Z

@Ivanidzo4ka I did before but this time I forgot to merge submodules. Sorry! Now it's merged as well 👍

Ivanidzo4ka · 2018-11-28T20:30:31Z

src/Microsoft.ML.Data/Evaluators/Metrics/RankerMetrics.cs

+    public sealed class RankerMetrics
+    {
+        /// <summary>
+        /// Array of normalized discounted cumulative gains where i-th element represent NDCG@i (you can also phrase it as NDCG at i).


(you can also phrase it as NDCG at i) [](start = 96, length = 37)

You don't have to write this. I just try to give you option how you can phrase it :)
it's either NDCG@i or NGDCG at i, both a valid names.

robosek · 2018-11-28T20:47:12Z

@Ivanidzo4ka of course. Done 😛

Reviewer is not available now, the PR is updated.

sfilipi

robosek added 4 commits November 17, 2018 19:03

Move evaluation results to stand alone metrics classes

9f5a8b7

Update metrics types

af8b8e1

Uppercase 'C' in MultiClassClassifierEvaluator and Metrics to keep co…

c8eb287

…nsistency

Fix metrics types

ab3ddee

TomFinley requested review from Zruty0 and sfilipi and removed request for Zruty0 November 18, 2018 02:35

Zruty0 previously requested changes Nov 19, 2018

View reviewed changes

robosek added 3 commits November 19, 2018 19:33

Merge branch 'master' into metrics-classes

0fdc184

Change namespace to Microsoft.ML.Data

3a94abd

Adjust metrics namespaces

61b2941

sfilipi reviewed Nov 20, 2018

View reviewed changes

robosek added 2 commits November 20, 2018 18:48

XMl encode apostrophes

bd04950

Merge branch 'master' into metrics-classes

db3d08f

sfilipi requested review from Ivanidzo4ka and TomFinley November 27, 2018 21:27

Ivanidzo4ka reviewed Nov 27, 2018

View reviewed changes

Ivanidzo4ka approved these changes Nov 27, 2018

View reviewed changes

robosek added 2 commits November 28, 2018 19:02

Fix conflicts and merge master

9561045

Use metric types

ece2b6e

merge submodules

ce5b416

Ivanidzo4ka reviewed Nov 28, 2018

View reviewed changes

Fix descriptions

33d35f4

sfilipi approved these changes Nov 29, 2018

View reviewed changes

sfilipi merged commit c3a20fa into dotnet:master Nov 29, 2018

ghost locked as resolved and limited conversation to collaborators Mar 26, 2022

Move evaluation results to separate classes #1663

Move evaluation results to separate classes #1663

Uh oh!

Conversation

robosek commented Nov 17, 2018

Uh oh!

Zruty0 left a comment

Choose a reason for hiding this comment

Uh oh!

robosek commented Nov 19, 2018 • edited by sfilipi Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Zruty0 commented Nov 19, 2018 • edited by sfilipi Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sfilipi Nov 20, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Ivanidzo4ka Nov 27, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Ivanidzo4ka Nov 27, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Ivanidzo4ka Nov 27, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Ivanidzo4ka left a comment

Choose a reason for hiding this comment

Uh oh!

Ivanidzo4ka commented Nov 27, 2018 • edited by sfilipi Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

robosek commented Nov 28, 2018 • edited by sfilipi Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Ivanidzo4ka commented Nov 28, 2018

Uh oh!

robosek commented Nov 28, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

robosek commented Nov 28, 2018

Uh oh!

sfilipi left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

robosek commented Nov 19, 2018 •

edited by sfilipi

Loading

Zruty0 commented Nov 19, 2018 •

edited by sfilipi

Loading

sfilipi Nov 20, 2018 •

edited

Loading

Ivanidzo4ka Nov 27, 2018 •

edited

Loading

Ivanidzo4ka Nov 27, 2018 •

edited

Loading

Ivanidzo4ka Nov 27, 2018 •

edited

Loading

Ivanidzo4ka commented Nov 27, 2018 •

edited by sfilipi

Loading

robosek commented Nov 28, 2018 •

edited by sfilipi

Loading