-
Notifications
You must be signed in to change notification settings - Fork 3.6k
fix(asr): repair ConvSubsampling forward paths missed by the MaskedConvSequential refactor #16225
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.
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -492,7 +492,13 @@ def forward(self, x, lengths): | |
| else: | ||
| x, lengths = self.conv(x, lengths) | ||
| else: | ||
| x, lengths = self.conv(x) | ||
| if self.conv2d_subsampling: | ||
| # Chunking disabled (-1): run the masked conv stack directly. | ||
| x, lengths = self.conv(x, lengths) | ||
| else: | ||
| # 1-D conv stacks run without masking; keep the lengths computed above. | ||
| x = self.conv(x) | ||
| lengths = out_lengths | ||
|
|
||
| # Flatten Channel and Frequency Axes | ||
| if self.conv2d_subsampling: | ||
|
|
@@ -725,7 +731,15 @@ class MaskedConvSequential(nn.Sequential): | |
| # Set by ConvSubsampling; off by default, so every other subsampling type stays on PyTorch. | ||
| fuse_triton = False | ||
|
|
||
| def forward(self, x, lengths): | ||
| def forward(self, x, lengths=None): | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Commit AGENTS.md reference: AGENTS.md:L80-L90 Useful? React with 👍 / 👎. |
||
| if lengths is None: | ||
| # Plain pass-through, equivalent to nn.Sequential. Used by the 1-D conv | ||
| # stacks (striding_conv1d / dw_striding_conv1d), whose inputs are already | ||
| # channels-first and which do not take part in length masking. | ||
| for layer in self: | ||
| x = layer(x) | ||
| return x | ||
|
|
||
| # Convert input (batch, time, features) to conv format | ||
| x = x.unsqueeze(1) # (batch, 1, time, features) | ||
| current_lengths = lengths | ||
|
|
@@ -760,10 +774,22 @@ def _forward_torch(self, x, current_lengths): | |
| x = layer(x) | ||
|
|
||
| # Update lengths for stride operations with proper padding | ||
| if hasattr(layer, 'stride') and layer.stride != (1, 1): | ||
| current_lengths = calculate_conv_output_size( | ||
| current_lengths, layer.kernel_size[0], layer.stride[0], _layer_padding(layer) | ||
| ) | ||
| if hasattr(layer, 'stride') and _pair_first(layer.stride) != 1: | ||
| kernel_size = _pair_first(layer.kernel_size) | ||
| stride = _pair_first(layer.stride) | ||
| left_pad, right_pad = _layer_padding(layer) | ||
| if getattr(layer, 'ceil_mode', False): | ||
| # Ceil-mode pooling (e.g. vggnet's MaxPool2d) emits one extra frame | ||
| # whenever the floor division drops a remainder. | ||
| remainder = (current_lengths + left_pad + right_pad - kernel_size) % stride | ||
| current_lengths = calculate_conv_output_size( | ||
| current_lengths, kernel_size, stride, (left_pad, right_pad) | ||
| ) | ||
| current_lengths = current_lengths + (remainder != 0).long() | ||
| else: | ||
| current_lengths = calculate_conv_output_size( | ||
| current_lengths, kernel_size, stride, (left_pad, right_pad) | ||
| ) | ||
| mask = self._create_mask(x, current_lengths.long()) | ||
|
|
||
| return x, current_lengths, mask | ||
|
|
@@ -826,15 +852,26 @@ def _create_mask(self, tensor, lengths): | |
| return time_mask.unsqueeze(-1).expand(batch_size, time, features).to(tensor.dtype) | ||
|
|
||
|
|
||
| def _pair_first(value): | ||
| """First element of an int-or-tuple kernel/stride/padding attribute. | ||
|
|
||
| nn.Conv2d stores tuples, but the pooling modules (nn.MaxPool2d/nn.AvgPool2d) keep | ||
| whatever was passed in, which ConvSubsampling passes as a plain int. | ||
| """ | ||
| return value[0] if isinstance(value, tuple) else value | ||
|
|
||
|
|
||
| def _layer_padding(layer): | ||
| """The (start, end) padding of a convolution. | ||
|
|
||
| nn.Conv2d's `.padding` is (pad_h, pad_w), one value per axis and symmetric within it, so the | ||
| height value is both edges. CausalConv2D keeps its two edges on private attributes. | ||
| nn.Conv2d's `.padding` is (pad_h, pad_w), one value per axis and symmetric within it, so | ||
| the height value is both edges. CausalConv2D keeps its two edges on private attributes. | ||
| Pooling layers store an int (or an int-per-axis tuple), which is symmetric too. | ||
| """ | ||
| if hasattr(layer, "_left_padding"): | ||
| return layer._left_padding, layer._right_padding | ||
| return layer.padding[0], layer.padding[0] | ||
| padding = _pair_first(layer.padding) | ||
| return padding, padding | ||
|
|
||
|
|
||
| def _is_depthwise(layer): | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For
striding_conv1danddw_striding_conv1dwith variable-length batches, this call deliberately omitslengths, so the newlengths=Nonebranch processes padded frames as real input. With the symmetric stride-2 convolutions, padded values can affect even the final frame insideout_lengths(for example, the last valid output for length 97 and kernel size 5 reads positions beyond 96), silently making model results depend on batch padding. The new tests assert only shapes and calculated lengths, so they do not detect this; implement length-aware mask propagation for the 1-D layers and test invariance to changes beyond each sample's length.AGENTS.md reference: AGENTS.md:L72-L78
Useful? React with 👍 / 👎.