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

Enables FastTreeBinaryClassificationCategoricalSplitTest and BinaryClassifierTesterThresholdingTest #255

Merged
merged 7 commits into from
May 30, 2018

Conversation

Anipik
Copy link
Contributor

@Anipik Anipik commented May 28, 2018

Test Being Enabled - BinaryClassifierTesterThresholdingTest, FastTreeBinaryClassificationCategoricalSplitTest

Zbaselines used by BinaryClassifierTesterThresholdingTest are already present. The slight difference in numbers is due to change in underlying framework

FastTreeBinaryClassificationCategoricalSplitTest

Learners Used :- FastTreeClassfier, FastTreeWithCategoricalClassfier, FastTreeClassfierDisk, FastTreeWithCategoricalClassfierDisk

Datasets Used:- adultOnlyCat, adult

Files BreakDown For FastTreeBinaryClassificationCategoricalSplitTest :- 40 release files , 40 debug files
Each Learner needs 10 files (5 For each datasets) The 5 files are *.txt, .out.txt, .rp.txt, *.ini, summary.txt

Note :- Release files are exactly same as Debug files

cc @shauheen @Ivanidzo4ka @eerhardt @danmosemsft @codemzs

@Anipik Anipik changed the title Thresh hold Enables FastTreeBinaryClassificationCategoricalSplitTest and BinaryClassifierTesterThresholdingTest May 28, 2018
@@ -204,8 +204,8 @@ public static class TestDatasets
public static TestDataset adult = new TestDataset
{
name = "Census",
trainFilename = @"..\UCI\adult.train",
testFilename = @"..\UCI\adult.test",
trainFilename = @"adult.train",
Copy link
Contributor

Choose a reason for hiding this comment

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

@ [](start = 28, length = 1)

Thanks for moving the file! This is an improvement.

Now that we no longer have a path hard coded we can remove the @.

Copy link
Contributor

Choose a reason for hiding this comment

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

Here and everywhere, just to be clear.


In reply to: 191518400 [](ancestors = 191518400)

@Anipik
Copy link
Contributor Author

Anipik commented May 29, 2018

@dotnet-bot test OSX10.13 Debug

Copy link
Contributor

@TomFinley TomFinley left a comment

Choose a reason for hiding this comment

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

Great thanks much @Anipik !

@Anipik
Copy link
Contributor Author

Anipik commented May 29, 2018

@dotnet-bot test OSX10.13 Release

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:

@TomFinley
Copy link
Contributor

Hi @Anipik I elected to merge #263 first, since without that, that change would have merely had to have been larger. I think though you just need to move the ZBaseline files here and in #253 to get these working again.

@Anipik
Copy link
Contributor Author

Anipik commented May 30, 2018

@TomFinley I moved the files in both the PRs

@TomFinley TomFinley merged commit ee7a669 into dotnet:master May 30, 2018
eerhardt pushed a commit to eerhardt/machinelearning that referenced this pull request Jul 27, 2018
…assifierTesterThresholdingTest (dotnet#255)

* Tests Enabled & Dataset Moved to correct place in test\BaselineOutput
* Correcting path for adult data set for autoInference class, and removing @ from path
@Anipik Anipik deleted the threshHold branch October 10, 2018 18:23
@ghost ghost locked as resolved and limited conversation to collaborators Mar 30, 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.

3 participants