rwth-i6 / returnn_common

Common building blocks for RETURNN configs, such as models, training concepts, etc
7 stars 4 forks source link

`nn.dropout` should have `axis` mandatory #77

Closed albertz closed 2 years ago

albertz commented 2 years ago

Related: #69, #76

albertz commented 2 years ago

I'm not so sure anymore. Using the feature dim is very standard for dropout.

You might argue that we never should use this concept of default feature dim. However, it is used in many places, even e.g. nn.Linear (#75). Also, as it was discussed before (https://github.com/rwth-i6/returnn/issues/586#issuecomment-969367369), maybe having a feature dim is actually not too much a problem. Except for those rare cases where it can become ambiguous but then we should just require those to be non-ambiguous.

But this discussion does not really belong here but to #17 instead.

I just say, if we make use in some places, e.g. also nn.Linear, then it should be ok here as well.

(@Zettelkasten opinions maybe?)

albertz commented 2 years ago

Well, actually I think we should just make it mandatory anyway. Done now.