Can we had n_filters to BaseDCUNet ?

[JorisCos]

❓ BaseDCUNet and DCCRN:

In the BaseDCUNet class which is also used for DCCRNet we assume that n_filters=stft_kernel_size.

 def __init__(
        self,
        architecture,
        stft_kernel_size=512,
        stft_stride=None,
        sample_rate=16000.0,
        **masknet_kwargs,
    ):
        self.architecture = architecture
        self.stft_kernel_size = stft_kernel_size
        self.stft_stride = stft_stride
        self.masknet_kwargs = masknet_kwargs

        encoder, decoder = make_enc_dec(
            "stft",
            kernel_size=stft_kernel_size,
            n_filters=stft_kernel_size,
            stride=stft_stride,
            sample_rate=sample_rate,
        )

In the DCCRN paper it's said

3.2. Training setup and baselines
For all of the models, the window length and hop size are 25 msand 6.25 ms, and the FFT length is 512.

To match the paper design, can we add n_filters as an argument of BaseDCUNet ?

[jonashaag]

Sure, why not, I'd also fb_name (see asteroid-team/asteroid-filterbanks#6)

[JorisCos]

Then we should remove the stft from self.stft_kernel_size and self.stft_stride and put 'stft' as default for fb_name.

[mpariente]

Let's start with adding stft_n_filters only because it's backward compatible. We'll see later about adding fb_name, because it has other implications (as said by @JorisCos)

[jonashaag]

I was just working on fixing the #406 failures when I realised that we're going to break backwards compat there anyways (with the #400 fix). So, two suggestions:

• Only add stft_n_filters without breaking anything, and not fixing Proper STFT configuration for DCUNet #400
• Fix Proper STFT configuration for DCUNet #400, also fix DCCRNet to the paper values, and also change filterbank options so that you have free choice of filterbank. The latter we could make backwards compatible because of the stft_ prefix that we currently have

[mpariente]

If you feel like going for option 2, that would be nice !

[jonashaag]

Hmm... the models already accept **masknet_kwargs, so how are we going to add the filterbank params?

1. Using an extra hierarchy for encoder params, eg. encoder={"kernel_size": ...} dictionary
2. Explicitly listing any supported filterbank params (eg. a common subset of params used in most filterbanks)

Other ideas?

[mpariente]

Option 2 is ok I guess. Otherwise, the user can instantiate the EMD model by hand with the encoder/decoder of choice.

[mpariente]

What's important is that the common use case is easy, and passing a dict is ugly, it's not worth it.

[mpariente]

I realize now that we did neither 1 or 2. We broke backward compat with the arguments but restricted the STFT.
I'll close, because adding fb_name was not the purpose of this issue.