escnn icon indicating copy to clipboard operation
escnn copied to clipboard

Give devs the flexibility to control PointwiseAvgPoolAntialiased2D output

Open psteinb opened this issue 1 year ago • 1 comments

Just bumped my head into this issue for a while during debugging. It would be great if the implicit kernel_size could be made an explicit part of the API of PointwiseAvgPoolAntialiased2D and others: https://github.com/QUVA-Lab/escnn/blob/8795704d57f492e354cd051ca74b825f9b06e9a1/escnn/nn/modules/pooling/pointwise_avg.py#L132

    def __init__(self,
                 in_type: FieldType,
                 sigma: float,
                 stride: Union[int, Tuple[int, int]],
                 # kernel_size: Union[int, Tuple[int, int]] = None, #<- this provides only rigid control of this class
                 padding: Union[int, Tuple[int, int]] = None,
                 #dilation: Union[int, Tuple[int, int]] = 1,
                 ):

I suggest to make kernel_size part of the API again and fill filter with 0 for example. This would give downstream users the freedom to play more with the output of the underlying conv operation.

psteinb avatar Mar 03 '23 16:03 psteinb

Hi @psteinb

I am not completely convinced by this solution, to be honest.

First, at the moment, the kernel_size is chosen to fit best the antialiasing filter. I feel like allowing the user to set a different kernel_size would make this module less reliable, since a bad choice of kernel_size will break the antialiasing filter without the user noticing.

Second, the PointwiseMaxPoolAntialiased2D module already has a kernel_size argument which, however, has a different meaning (it refers to the size of the window used to compute the max). I fear having a kernel_size argument in the average pooling would be confusing.

I will probably use a different naming for this kernel_size and raise a warning if it is too small. I'll try to implement this in the next days, but feel free to open a PR if you already did something like that

Best, Gabriele

Gabri95 avatar Mar 08 '23 17:03 Gabri95