Skip to content

Conversation

@kere-nel
Copy link
Contributor

No description provided.

@codecov
Copy link

codecov bot commented Jun 17, 2020

Codecov Report

Merging #5246 into master will increase coverage by 0.04%.
The diff coverage is 85.71%.

@@            Coverage Diff             @@
##           master    #5246      +/-   ##
==========================================
+ Coverage   73.51%   73.55%   +0.04%     
==========================================
  Files        1013     1022       +9     
  Lines      188446   189693    +1247     
  Branches    20289    20441     +152     
==========================================
+ Hits       138528   139534    +1006     
- Misses      44426    44619     +193     
- Partials     5492     5540      +48     
Flag Coverage Δ
#Debug 73.55% <85.71%> (+0.04%) ⬆️
#production 69.34% <66.66%> (+0.03%) ⬆️
#test 87.51% <100.00%> (+0.08%) ⬆️
Impacted Files Coverage Δ
...L.AutoML/TrainerExtensions/TrainerExtensionUtil.cs 84.71% <ø> (ø)
src/Microsoft.ML.AutoML/Utils/BestResultUtil.cs 53.84% <0.00%> (ø)
src/Microsoft.ML.AutoML/API/RankingExperiment.cs 60.60% <60.00%> (+2.54%) ⬆️
...ML/Experiment/MetricsAgents/RankingMetricsAgent.cs 65.51% <100.00%> (+1.23%) ⬆️
test/Microsoft.ML.AutoML.Tests/AutoFitTests.cs 90.30% <100.00%> (ø)
...st/Microsoft.ML.AutoML.Tests/MetricsAgentsTests.cs 100.00% <100.00%> (ø)
...c/Microsoft.ML.FastTree/Utils/ThreadTaskManager.cs 79.48% <0.00%> (-20.52%) ⬇️
src/Microsoft.ML.FastTree/RegressionTree.cs 75.51% <0.00%> (-8.17%) ⬇️
src/Microsoft.ML.LightGbm/LightGbmTrainerBase.cs 78.92% <0.00%> (-6.07%) ⬇️
src/Microsoft.ML.AutoML/API/AutoCatalog.cs 69.35% <0.00%> (-4.84%) ⬇️
... and 28 more

}

public RankingMetrics EvaluateMetrics(IDataView data, string labelColumn)
public RankingMetrics EvaluateMetrics(IDataView data, string labelColumn, string groupIdColumn)
Copy link
Member

Choose a reason for hiding this comment

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

Can we just put groupId column as an option when creating RankingMetricsAgent? So that we don't have to add an extra groupIdColumn to IAgent.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, we can do that. I think a possible concern is that GroupId is required by RankingTrainers, and passing it in as an option gives me the impression that it's optional. I think that if we make the default value of GroupId "GroupId" and document that, it might be okay. What you do think?

@kere-nel kere-nel marked this pull request as ready for review June 29, 2020 18:14
@kere-nel kere-nel requested a review from a team as a code owner June 29, 2020 18:14
@kere-nel kere-nel merged commit dd81e79 into dotnet:master Jun 30, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Mar 18, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants