-
Notifications
You must be signed in to change notification settings - Fork 6.8k
Dropout may mask values even when ratio=0.0 #9816
Comments
This needs to be fix. |
I like making ratio=0 a special case with an identity pass-through. During the same fix, how about making ratio=1 a special case also to output all 0's (current behavior is to pass inf or nan I think)? |
There was an issue in mshadow about this dmlc/mshadow#213. The cuda rng generates (0.0, 1.0]. |
@DickJC123 re-running the test with your scenario appears to be working for me (today). Are you still experiencing a problem or can we close this issue? |
@samskalicky Able to reproduce the issue with seeds 111913613, 211508467, 1329041279
|
On running this on CPU, the flaky error is reproducible, as mentioned in the above comments.
On running on GPU with the failure seeds for CPU : 111913613, 211508467, 1329041279, the test still does not fail.
|
Bug can be worked around by adding a threshold_eq (<=) operator in mshadow_op.h and calling that one instead in dropout-inl.h Tested working with seed 111913613 |
* added mshadow op for threshold_eq (theshold currently does <, this will do <=) modified dropout operator to use threshold_eq instead of theshold this will ensure equivalent behavior for the random numbers generated on CPU [0, 1) and GPU (0, 1] removed fixed seed for test_dropout * removed comment about flaky test
@sandeep-krishnamurthy Related fix for this issue is merged, this issue should be good to close. |
…pache#12091) * added mshadow op for threshold_eq (theshold currently does <, this will do <=) modified dropout operator to use threshold_eq instead of theshold this will ensure equivalent behavior for the random numbers generated on CPU [0, 1) and GPU (0, 1] removed fixed seed for test_dropout * removed comment about flaky test
…pache#12091) * added mshadow op for threshold_eq (theshold currently does <, this will do <=) modified dropout operator to use threshold_eq instead of theshold this will ensure equivalent behavior for the random numbers generated on CPU [0, 1) and GPU (0, 1] removed fixed seed for test_dropout * removed comment about flaky test
Unfortunately the issue with dropout operator is not fully solved - when using seed 579061237 on the GPU, rng produces 0 for one of the outputs in ratio=1.0 case, which, because of the usage of <=, ends up being |
One reasonable expectation is that a dropout layout would pass no values when the dropout ratio=1.0 and would pass all values with the dropout ratio=0.0. However, the current dropout test fails a ratio=0.0 test under some seeds because some values are masked. Adapting from the current nn/dropout-inl.h, we have essentially:
It must be that rand_pick above can include the 1.0 endpoint, so mask_out becomes 0.0. A quick fix might change the '<' to '<=', but then some values might be passed when the dropout ratio = 1.0 if the rand_pick can be 0.0. The possible differences between rng properties between cpu and gpu's should be considered: CUDNN rngs return values in (0.0,1.0] while most others are in the range [0.0,1.0). This can be reproduced in any of the commits of the ci_test_randomness3 PR #9791 if you edit the line above 'def test_dropout()' in test_operator.py to be only "@with_seed()", then execute: (output shown)
The text was updated successfully, but these errors were encountered: