Skip to content

Adding defaults for Label and GroupId to Ranking Evaluator #2684

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

Merged

Conversation

rogancarr
Copy link
Contributor

This PR adds default column names for Label and GroupId for the Ranking Evaluator, in line with all other Evaluators.

Fixes #2633

@codecov
Copy link

codecov bot commented Feb 21, 2019

Codecov Report

Merging #2684 into master will decrease coverage by <.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #2684      +/-   ##
==========================================
- Coverage   71.54%   71.54%   -0.01%     
==========================================
  Files         800      800              
  Lines      141827   141827              
  Branches    16119    16119              
==========================================
- Hits       101472   101470       -2     
- Misses      35913    35915       +2     
  Partials     4442     4442
Flag Coverage Δ
#Debug 71.54% <ø> (-0.01%) ⬇️
#production 67.85% <ø> (ø) ⬆️
#test 85.7% <ø> (-0.02%) ⬇️
Impacted Files Coverage Δ
src/Microsoft.ML.Data/TrainCatalog.cs 90.08% <ø> (ø) ⬆️
src/Microsoft.ML.Maml/MAML.cs 24.75% <0%> (-1.46%) ⬇️
...soft.ML.TestFramework/DataPipe/TestDataPipeBase.cs 73.76% <0%> (-0.33%) ⬇️
...soft.ML.Data/DataLoadSave/Text/TextLoaderCursor.cs 83.06% <0%> (ø) ⬆️
src/Microsoft.ML.Data/Training/TrainerUtils.cs 71.57% <0%> (+0.79%) ⬆️

@danmoseley
Copy link
Member

@safern do you know what the magic is to make all the legs show up above in github, like they do now in CoreFX, rather than just one meta leg "MachineLearning-CI"

@safern
Copy link
Member

safern commented Feb 21, 2019

@safern do you know what the magic is to make all the legs show up above in github, like they do now in CoreFX, rather than just one meta leg "MachineLearning-CI"

We need to change the connection settings in the build definition itself. That will also enable comment triggers for this repo. @eerhardt @shauheen I can take care of that, I think using the new connection setting is worth it as it also gets a lot of fixes from the azure devops CI team. However, it adds an extra step to get to the builds. When you click on details, it sends you to the checks tab in GH.

@eerhardt
Copy link
Member

I'm not sure if we need anything different than what we have today... I feel I'm productive in the current environment.

But if others feel it is valuable to have something different, I won't object unless I think that something different is worse than what we have today.

@safern
Copy link
Member

safern commented Feb 22, 2019

But if others feel it is valuable to have something different, I won't object unless I think that something different is worse than what we have today.

To just feel the difference you can go into a corefx PR and look at the builds, click on one of them and try to navigate to it, etc.

@eerhardt
Copy link
Member

Yea, that experience is worse IMO.

@safern
Copy link
Member

safern commented Feb 22, 2019

Yea, that experience is worse IMO.

Yeah, but I think is getting cool feature, like retrying a single job, rather than the whole pipeline, and comment triggers, optional builds triggered by comments, etc. Stuff corefx did require.

Ivanidzo4ka
Ivanidzo4ka previously approved these changes Feb 22, 2019
Copy link
Contributor

@Ivanidzo4ka Ivanidzo4ka left a comment

Choose a reason for hiding this comment

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

:shipit:

@Ivanidzo4ka Ivanidzo4ka dismissed their stale review February 22, 2019 19:24

revoking review

Copy link
Contributor

@Ivanidzo4ka Ivanidzo4ka left a comment

Choose a reason for hiding this comment

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

:shipit:

@rogancarr rogancarr requested a review from glebuk February 22, 2019 20:42
@Ivanidzo4ka Ivanidzo4ka requested a review from artidoro February 22, 2019 21:17
@rogancarr rogancarr merged commit eb959c3 into dotnet:master Feb 22, 2019
@rogancarr rogancarr deleted the 2633_add_defaults_to_ranking_evaluator branch February 22, 2019 23:59
@ghost ghost locked as resolved and limited conversation to collaborators Mar 24, 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.

6 participants