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

Spurious Failures on TestPfiClusteringOnDenseFeatures test #1925

Closed
TomFinley opened this issue Dec 19, 2018 · 1 comment
Closed

Spurious Failures on TestPfiClusteringOnDenseFeatures test #1925

TomFinley opened this issue Dec 19, 2018 · 1 comment
Assignees
Labels
test related to tests
Projects
Milestone

Comments

@TomFinley
Copy link
Contributor

TestPfiClusteringOnDenseFeatures has, I believe introduced a week ago in PR #1832, has been responsible for some seemingly spurious test failures that are blocking builds. See, e.g., this build here from @Zruty0 for his PR #1920 , where PFI (which is totally unrelated to @Zruty0 's change) is blocking checking it in.

My best guess as to why this is happening is that for this clustering test the number of threads was not set to 1. (That obviously will cause issues, but whether it's the only issue is more than I know right now.) Sometimes you can kinda get away with that (e.g., sometimes results from logistic regression can resemble each other in different configuration settings since it is solving a convex problem), but this is not true of clustering. For that reason, the test TestPfiClusteringOnDenseFeatures has been responsible for multiple spurious test failures since its introduction last week.

We will have a PR to set the threads to 1, at least as a first try to get the situation somewhat under control. Note that this will not be necessary in FastTree since it is engineered in such a way that it gets the same result no matter how many or few threads are used.

/cc @rogancarr @shmoradims @artidoro

@TomFinley TomFinley added the test related to tests label Dec 19, 2018
@TomFinley TomFinley added this to the 1218 milestone Dec 19, 2018
@TomFinley TomFinley self-assigned this Dec 19, 2018
@TomFinley TomFinley added this to Done in v0.9 via automation Dec 19, 2018
@rogancarr
Copy link
Contributor

@TomFinley #1844 removes PFI clustering -- it doesn't really make sense for this learning task.

@ghost ghost locked as resolved and limited conversation to collaborators Mar 26, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
test related to tests
Projects
No open projects
v0.9
  
Done
Development

No branches or pull requests

2 participants