Skip to content

Fix flakiness on StochasticDepth test #4758

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.

Sign up for GitHub

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

Merged
merged 4 commits into from
Oct 27, 2021
Merged

Fix flakiness on StochasticDepth test #4758

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
932bcc2
Fix flakiness on the TestStochasticDepth test.
datumbox Oct 27, 2021
34886ca
Fix minor bug when p=1.0
datumbox Oct 27, 2021
b445bfb
Remove device and dtype setting.
datumbox Oct 27, 2021
59c1c8b
Merge branch 'main' into tests/flaky_stochastic_depth
datumbox Oct 27, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 20 additions & 3 deletions test/test_ops.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1149,13 +1149,15 @@ def _create_masks(image, masks):


class TestStochasticDepth:
@pytest.mark.parametrize("seed", range(10))
@pytest.mark.parametrize("p", [0.2, 0.5, 0.8])
@pytest.mark.parametrize("mode", ["batch", "row"])
def test_stochastic_depth(self, mode, p):
def test_stochastic_depth_random(self, seed, mode, p):
torch.manual_seed(seed)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We maintain the original test because it allows us to check that the different mode values operate as expected. Using p values in the interval (0, 1) is critical because for p=0 and p=1 the two modes behave the same. The mitigation for the flakiness here is to set the seed.

Copy link
Member

Choose a reason for hiding this comment

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

On my laptop the test passed over 97/100 seeds so there's a very small change it will fail in the future. (first failure on the 11th seed lol)

I agree it's fine to leave as-is for now

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's because I increased the p-value threshold to 1%. You kind of expect false positives around that rate now, hopefully we won't see it due to the seed setting.

stats = pytest.importorskip("scipy.stats")
batch_size = 5
x = torch.ones(size=(batch_size, 3, 4, 4))
layer = ops.StochasticDepth(p=p, mode=mode).to(device=x.device, dtype=x.dtype)
layer = ops.StochasticDepth(p=p, mode=mode)
layer.__repr__()

trials = 250
Expand All @@ -1173,7 +1175,22 @@ def test_stochastic_depth(self, mode, p):
num_samples += batch_size

p_value = stats.binom_test(counts, num_samples, p=p)
assert p_value > 0.0001
assert p_value > 0.01
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Significantly increase the threshold since now we only check 10 seeds. We can reduce if flakiness continues.


@pytest.mark.parametrize("seed", range(10))
@pytest.mark.parametrize("p", (0, 1))
@pytest.mark.parametrize("mode", ["batch", "row"])
def test_stochastic_depth(self, seed, mode, p):
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Adding an additional test with p=0 and p=1 to confirm it works as expected for the extreme values.

torch.manual_seed(seed)
batch_size = 5
x = torch.ones(size=(batch_size, 3, 4, 4))
layer = ops.StochasticDepth(p=p, mode=mode)

out = layer(x)
if p == 0:
assert out.equal(x)
elif p == 1:
assert out.equal(torch.zeros_like(x))


class TestUtils:
Expand Down
4 changes: 3 additions & 1 deletion torchvision/ops/stochastic_depth.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,9 @@ def stochastic_depth(input: Tensor, p: float, mode: str, training: bool = True)
else:
size = [1] * input.ndim
noise = torch.empty(size, dtype=input.dtype, device=input.device)
noise = noise.bernoulli_(survival_rate).div_(survival_rate)
noise = noise.bernoulli_(survival_rate)
if survival_rate > 0.0:
noise.div_(survival_rate)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was actually a bug; the previous code produced nans! Though it really isn't something that users will face. Setting p=1 to the operator means that you will always drop the block (set it to 0). I'm not sure that this is something many users would like to do, but worth fixing anyway.

return input * noise


Expand Down