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.

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

Enabling custom groupId column in the Ranking AutoML experiment #5246

Merged
merged 3 commits into from
Jun 30, 2020

Conversation

Lynx1820
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

@@ -57,9 +57,9 @@ public bool IsModelPerfect(double score)
}
}

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

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?

@Lynx1820 Lynx1820 marked this pull request as ready for review June 29, 2020 18:14
@Lynx1820 Lynx1820 requested a review from a team as a code owner June 29, 2020 18:14
@Lynx1820 Lynx1820 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