snorkel-team / snorkel

A system for quickly generating training data with weak supervision
https://snorkel.org
Apache License 2.0
5.81k stars 857 forks source link

Fixing flaky test #1671

Closed crawlingcub closed 2 years ago

crawlingcub commented 3 years ago

The test test_labeling_convergence in test/labeling/test_convergence.py sometimes fails when the seed settings are removed. It failed 52 out of 500 runs times for me on my machine. Updating the assertion threshold to 0.06 ensures that the test always passes (even without seeds). For obtaining this solution, I analyzed the underlying tail distribution of the err variable and computed the 99.99th percentile. This provided an estimate of ~0.0555. I rounded up the value in the assertion to fix the test.

I think the seed settings on lines 61-63 can also be removed since this class only has one test. I can remove them if you agree with this change.

Please let me know if this change looks reasonable. I will be happy to incorporate any other suggestions that you may have.

Thanks!

github-actions[bot] commented 2 years ago

This pull request is stale because it has been open 90 days with no activity. Remove stale label or comment or this will be closed in 7 days.

github-actions[bot] commented 2 years ago

This pull request is stale because it has been open 90 days with no activity. Remove stale label or comment or this will be closed in 7 days.

fpoms commented 2 years ago

@crawlingcub thanks for the contribution--and sorry for the delay here. I've confirmed this works as you described am ok to merge if that's ok with you!

fpoms commented 2 years ago

@crawlingcub if you could rebase onto main, and the tests pass, then I can merge this.

crawlingcub commented 2 years ago

hi i merged master. can you merge?

fpoms commented 2 years ago

Merged! Thanks @crawlingcub !