executorch icon indicating copy to clipboard operation
executorch copied to clipboard

Inappropriate detection of dynamic dimensions in executorch/backends/xnnpack/operators/op_squeeze.py

Open ezyang opened this issue 1 year ago • 1 comments

🐛 Describe the bug

You have this logic:

        num_dynamic_dims = 0
        for dim in dynamic_shape:
            if isinstance(dim, torch.SymInt):
                num_dynamic_dims += 1
                new_shape.append(0)
            else:
                new_shape.append(dim)

        check_or_raise(
            num_dynamic_dims <= 1,
            "XNNPACK reshape only supports 1 dynamic dimension. This may occur when ",
        )

This is not the right way to detect how many dynamic dimensions are going into the squeeze call. For one, you could potentially have a SymInt which internally contains a constant int (we try to decay this ASAP, but if you have a SymInt in hand and then later we do a replacement that refines it to an int, you are still going to have a SymInt.)

I think probably what's more right is to call free_symbols(dim). When there are no free symbols, you are convertible to int, which is what this code is trying to do right?

Versions

main

ezyang avatar Oct 01 '24 18:10 ezyang

cc @mcr229 @digantdesai

ezyang avatar Oct 02 '24 02:10 ezyang

Fixed.

digantdesai avatar Feb 07 '25 22:02 digantdesai