Batch processing implementation for Roformer models: Seeking feedback on potential pull request

[ntamotsu]

Hello,

I noticed that the batch_size parameter wasn't being utilized for Roformer models. To address this, I've implemented batch processing for these models. I'd like to share my findings and seek your feedback on whether this change would be beneficial to incorporate into the main repository.

Key points:

1. The original code didn't use the batch_size parameter for Roformer models, resulting in no batch processing.

2. I've implemented batch processing for Roformer models, allowing the batch_size parameter to be used effectively.

3. After implementation, I confirmed that setting larger batch_size values increases GPU VRAM usage for Roformer models.

4. Interestingly, I observed little to no improvement in processing speed, despite the batch processing implementation.

5. The README mentions that the batch_size parameter "may process slightly faster" for other models, so this behavior might be expected.

I've created a modified version of the demix function in the MDXCSeparator class to implement this change. You can find the updated code here:
https://github.com/ntamotsu/fork_python-audio-separator/blob/e44834b4d05203b82c61fb8c6b06205edde26276/audio_separator/separator/architectures/mdxc_separator.py#L248-L282)

Given these observations, I'm wondering if this change would be valuable to include in the main repository. If the lack of batch processing for Roformer models was intentional, then this modification may not be necessary.

I'd greatly appreciate your thoughts on this matter. Should I proceed with creating a pull request for this change, or do you feel it's not needed at this time?

Thank you for your time and consideration.

[beveradb]

Thanks for thinking about this!
To be totally honest if there isn't much tangible benefit, I'm not sure it's worth it?

[ntamotsu]

Thank you for your response. I agree that without a tangible benefit, a major code change might not be necessary.

However, after further consideration, I think it might be helpful to add a comment in the code explaining why the batch_size parameter isn't currently used for Roformer models. This would provide clarity for users who might wonder about its effect.
For example, we could add something like:

def demix(self, mix: np.ndarray) -> dict:
    # ...

    if self.is_roformer:
        # Note: Currently, for Roformer models, `batch_size` is not utilized due to negligible performance improvements.
        # ...

This approach would improve documentation without changing the functionality. What do you think?

[beveradb]

Sounds good to me!

[ntamotsu]

Thanks! I'll submit a pull request then.