fixed wrong runtime shape inference for BatchNorm1dToQuantScaleBias #465
Conversation
There was a problem hiding this comment.
Hi,
Thanks for following up on your issue and opening this PR.
We are in the process of better defining how to contribute to Brevitas, and recently we added some guidelines regarding how to add new tests.
Would you be able to add some tests regarding this PR?
For example, checking the output shapes for different types of input (either 2D or 3D).
Let me know if there is something not clear.
Giuseppe
src/brevitas/nn/quant_bn.py
Outdated
| @@ -97,7 +97,7 @@ def __init__( | |||
| super(BatchNorm1dToQuantScaleBias, self).__init__( | |||
| num_features, | |||
| bias=True, | |||
| runtime_shape=(1, -1, 1), | |||
| runtime_shape=(1, -1), | |||
There was a problem hiding this comment.
I think this has to stay as it was before, i.e. (1, -1, 1).
The reason is that BatchNorm1d is supposed to handle both 2D and 3D inputs. In case you need specifically to use a 2D input, you could always specify the runtime shape when instantiating the class.
There was a problem hiding this comment.
Sure thing. I haven't really made any serious test cases before, so this will take some time (since I would have to read all the prerequisites), but I could definitely make some drafts.
There was a problem hiding this comment.
I have also made the changes that you requested in my dev branch, but how do I update the pull request? Do I start another pull request? Thank you
There was a problem hiding this comment.
Every change, including commits, that you do in your dev branch will be automatically added to this PR, so when you have the tests, just add them to the same dev branch
…m (1,-1) to (1, -1, 1), as requested by the maintainer of brevitas branch
Fixed issue where the output shape of
class BatchNorm1dToQuantScaleBiaswas giving an unexpected shape of[1, input_dim, batch_dim, input_dim]instead of[batch_dim, input_dim].The issue came from the fact that the class ultimately had a default value of
runtime_shape = (1, -1, 1, 1)when it should beruntime_shape = (1, -1).We had
runtime_shape = (1, -1, 1, 1)becauseclass BatchNorm1dToQuantScaleBiaswasn't properly passing down theruntime_shapeparameter toclass ScaleBias. That issue has been also fixed.Original issue from #450