search

rwth-i6 / pytorch-to-returnn

Make PyTorch code runnable within RETURNN
3 stars 6 forks source link

changed unsqueeze behavior #94

Closed vieting closed 2 years ago

vieting commented 2 years ago

I have a case with a (B, T) tensor which is unsqueezed to (B, 1, T) (adding feature dim for raw waveform), however, the RETURNN data of the output is (B, T, F), so F and T are assigned to the wrong axes. It seems that the behavior changed and the RETURNN axes are not handled correctly now (or this just did not become obvious before).

albertz commented 2 years ago

I don't see from this description why this is a problem?

vieting commented 2 years ago

Yes sorry, that was not clear. The tensor is unsqueezed to (B, 1, T), however, the RETURNN data of the output is (B, T, F), so F and T are assigned to the wrong axes. I'll update the description.

albertz commented 2 years ago

But why is this axis assignment a problem?

vieting commented 2 years ago

I have a ConvLayer afterwards and now we explicitly specify in_spatial_dims since #93. In the case here, I get 'in_spatial_dims': ['F'] and AssertionError: invalid in_spatial_dims [Dim{'time:data'[B]}], because feature dim axes are not allowd in in_spatial_dims.

vieting commented 2 years ago

The SplitDimsLayer is called with {'class': 'split_dims', 'from': 'data', 'axis': 'T', 'dims': [1, -1]}. I think it's wrong/unintended, that in SplitDimsLayer.get_out_data_from_opts shortly before the end, out.time_dim_axis is set to 1 instead of 2. This corresponds to the splitted dim 1 instead of -1 which would be the original time dim axis.

albertz commented 2 years ago

The SplitDimsLayer is called with {'class': 'split_dims', 'from': 'data', 'axis': 'T', 'dims': [1, -1]}. I think it's wrong/unintended, that in SplitDimsLayer.get_out_data_from_opts shortly before the end, out.time_dim_axis is set to 1 instead of 2. This corresponds to the splitted dim 1 instead of -1 which would be the original time dim axis.

What is data in this case?

But even if this behavior is maybe unexpected (and maybe should be fixed), I still don't understand why it is a problem.

albertz commented 2 years ago

because feature dim axes are not allowd in in_spatial_dims.

Why do you think so? No, this should not be the case.

albertz commented 2 years ago

I changed the behavior now to make it more consistent (https://github.com/rwth-i6/returnn/pull/914). However, I still think that it should also work without this change, and whatever problem you encountered is sth else.

vieting commented 2 years ago

Thanks for the fix in https://github.com/rwth-i6/returnn/pull/914! This indeed fixes my problem as well. I updated #95 to better reflect the issue I was facing, but with the current RETURNN master, all test pass now.