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

Fixed score column name and order bugs in CalibratorTransformer #5261

Merged
merged 5 commits into from
Jul 1, 2020

Conversation

mstfbl
Copy link
Contributor

@mstfbl mstfbl commented Jun 26, 2020

Fix #4700

As explained in #4700, the column order in which the Score column is given to PlattCalibratorTransformer exposes a bug in CalibratorTransformer where the name and the location of the score column is hard-coded. This PR fixes this hard-coding, where now the name and order of the score column given to CalibratorTransformer can be different than the default values.

I have also added unit tests to confirm these changes with respect to direct non-standard column name tests and Onnx Conversions.

@codecov
Copy link

codecov bot commented Jun 30, 2020

Codecov Report

Merging #5261 into master will increase coverage by 0.24%.
The diff coverage is 98.00%.

@@            Coverage Diff             @@
##           master    #5261      +/-   ##
==========================================
+ Coverage   73.49%   73.74%   +0.24%     
==========================================
  Files        1014     1022       +8     
  Lines      188680   190200    +1520     
  Branches    20330    20463     +133     
==========================================
+ Hits       138677   140266    +1589     
+ Misses      44493    44418      -75     
- Partials     5510     5516       +6     
Flag Coverage Δ
#Debug 73.74% <98.00%> (+0.24%) ⬆️
#production 69.51% <95.65%> (+0.21%) ⬆️
#test 87.63% <98.70%> (+0.19%) ⬆️
Impacted Files Coverage Δ
test/Microsoft.ML.Tests/OnnxConversionTest.cs 96.57% <92.30%> (-0.05%) ⬇️
.../Microsoft.ML.Data/Prediction/CalibratorCatalog.cs 93.81% <95.65%> (+0.37%) ⬆️
...ML.Tests/TrainerEstimators/CalibratorEstimators.cs 97.60% <100.00%> (+2.51%) ⬆️
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%) ⬇️
src/Microsoft.ML.TimeSeries/ExtensionsCatalog.cs 90.62% <0.00%> (-3.00%) ⬇️
...rosoft.ML.AutoML/ColumnInference/TextFileSample.cs 59.60% <0.00%> (-2.65%) ⬇️
src/Microsoft.ML.Maml/MAML.cs 23.78% <0.00%> (-2.43%) ⬇️
...osoft.ML.Functional.Tests/IntrospectiveTraining.cs 98.73% <0.00%> (-1.27%) ⬇️
... and 46 more

@mstfbl mstfbl marked this pull request as ready for review June 30, 2020 09:17
@mstfbl mstfbl requested a review from a team as a code owner June 30, 2020 09:17
@mstfbl mstfbl marked this pull request as draft June 30, 2020 22:07
@mstfbl mstfbl marked this pull request as ready for review July 1, 2020 11:10
@mstfbl mstfbl requested a review from antoniovs1029 July 1, 2020 16:58
Copy link
Member

@antoniovs1029 antoniovs1029 left a comment

Choose a reason for hiding this comment

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

Just left a minor nit comment

@mstfbl mstfbl merged commit 81357ba into dotnet:master Jul 1, 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.

Using PlattCalibratorTransformer with custon name for Score Column
2 participants