[ENH] Add possibility for pooling strides in TimeCNN

[flomerit]

Describe the bug

Steps/Code to reproduce the bug

Expected results

average_pooling1d_31 │ (None, 7, 6) │ 0 │
│ (AveragePooling1D) │ │

Actual results

我发现TimeCNNnet的池化操作并没有传入池化步长，导致池化步长等于avg_pool_size

Versions

No response

[TonyBagnall]

hi, I'm not that familiar, are you saying the average_pooling1d_31 layer should be (none,7,6)? @aadya940 and/or @hadifawaz1999 may be able to advise

[hadifawaz1999]

Hello, could you clarify more on whats the issue ?

[flomerit]

hi, I'm not that familiar, are you saying the average_pooling1d_31 layer should be (none,7,6)? @aadya940 and/or @hadifawaz1999 may be able to advise
Hello, could you clarify more on whats the issue ?

Yes, I feel there are issues with the outshape of the average pooling layer, according to the API description, "strides: int or list of int, default = 1.The strides of kernels in the convolution and max pooling layers, if not a list, the same strides are used for all layers. "So when strides=1, I expect the average output shape of the pooling layer =(7,6), with 7 =(8-avg_pool_size+1)//strides. So, when I went to look at the underlying code, I found the aeon \ networks \ _cnn py "conv = tf. Keras. The layers. The AveragePooling1D (pool_size = self. _avg_pool_size [i]) (conv）”The strides parameter is not passed in. So I think that the code should be changed to "conv = tf. Keras. The layers. AveragePooling1D (pool_size = self. _avg_pool_size [i], strides. = the self _strides [i]) (conv). That's probably right.

[hadifawaz1999]

Thank you for the details. So the strides do not impact the average pooling layer as you can see in the source code but its only for the convolution layer. The reason we did that is because the original implementation did not include strides for the average pooling layer

So this is why when setting strides it wont affect the output shape of avg pooling layers because this is how the network is proposed.

[TonyBagnall]

thanks for the answer @hadifawaz1999, I dont think this is a bug, more of a feature? :)

[hadifawaz1999]

thanks for the answer @hadifawaz1999, I dont think this is a bug, more of a feature? :)

Yes its a feature i would change the tagging to enhancement issue, happy to have this feature in, however i would prefer to separate strides form ones of conv, so keep the strides that already are in the network, and add a parameter called strides_pooling with the same setup default to None as the original paper

[flomerit]

I get it, and I'm excited to look forward to the enhanced version.

…

---Original--- From: "Ali El Hadi ISMAIL ***@***.***> Date: Thu, Nov 7, 2024 01:28 AM To: ***@***.***>; Cc: ***@***.******@***.***>; Subject: Re: [aeon-toolkit/aeon] [ENH] Add possibility for pooling strides inTimeCNN (Issue #2307) thanks for the answer @hadifawaz1999, I dont think this is a bug, more of a feature? :) Yes its a feature i would change the tagging to enhancement issue, happy to have this feature in, however i would prefer to separate strides form ones of conv, so keep the strides that already are in the network, and add a parameter called strides_pooling with the same setup default to None as the original paper — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you authored the thread.Message ID: ***@***.***>