Closed melonwater211 closed 2 years ago
Hi @rsmith49.
After running additional tests, we have determined that increasing n_epochs
to 50 and lr
to 0.002383698505238875 would better reduce flakiness. Additionally, we have kept the seed-setting code, as requested. Please let us know if anything else should be done to fix this test. We would be happy to raise PRs about other flaky tests. Thank you.
Hi @rsmith49. I would also like to mention that, with these new changes in place, the probability of failure when the seed-setting code is removed is ~0.08%. Please let us know if anything else should be done or if the PR is ready to merge. Thank you.
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.
Hi @melonwater211 ! Sorry for the late reply. We recently went through some CI upgrades for the checks to become more streamlined. However, because of this, existing PRs such as this one have the old CI checks still attached.
Would you mind rebasing to the new master branch and pushing this PR from there? You can accomplish that by
git pull
git rebase master
git push --force
Thanks!
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.
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.
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.
Closing this as #1715 has addressed the same changes
This PR is a followup to Issue #1654 (@bhancock8). We have changed
n_epochs
from 10 to 20. Please let us know if there is anything else that should be done with regards totest_convergence
. Additionally, there are several other tests that demonstrate similar behavior. Please let us know if we should send the details about those tests. Thank you.