Clean amalgamation ws and skip failing test #7678
Conversation
tests/python/unittest/test_loss.py
Outdated
| @@ -63,6 +63,9 @@ def get_net(num_hidden): | |||
|
|
|||
|
|
|||
| def test_ce_loss(): | |||
| # Skipping this test to have nightly build passing. | |||
| # There seems to be an issue and tracked at - issue 7677 | |||
| return | |||
There was a problem hiding this comment.
This is not how it should be done. Aston is fixing loss problems in #7659.
How frequent is this test failing.
There was a problem hiding this comment.
Roughly 15%. I do hear your concern but it doesn't look like there is a risk of this issue getting dropped. Do you have a better suggestion? Unreliable builds and flaky tests are hurting us in other ways.
There was a problem hiding this comment.
Drop the change on test_ce_loss test in this PR and work with @astonzhang to fix this test in his PR.
There was a problem hiding this comment.
#7659 proposed some change in the unittest
There was a problem hiding this comment.
@sandeep-krishnamurthy lets get @astonzhang 's PR in and keep this only for amalgamation test.
There was a problem hiding this comment.
The unittest fix is now available in a separate pr #7693
sandeep-krishnamurthy
force-pushed
the
fix_ci_2
branch
from
dcc3c38 to
ff4f061
Compare
|
Remove logic to skipping the failing test case. Thanks to Aston for fixing it. |
* Clean amalgamation ws and skip failing test * Undo skipping the failing test_ce_loss
* Clean amalgamation ws and skip failing test * Undo skipping the failing test_ce_loss
* Clean amalgamation ws and skip failing test * Undo skipping the failing test_ce_loss
@madjam