Skip to content

Add support for torch op conv1d #1753

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Open
fursund opened this issue Feb 3, 2023 · 8 comments · May be fixed by #2011
Open

Add support for torch op conv1d #1753

fursund opened this issue Feb 3, 2023 · 8 comments · May be fixed by #2011
Labels
feature request Functionality does not currently exist, would need to be created as a new feature (type)

Comments

@fursund
Copy link

fursund commented Feb 3, 2023

coremltools is missing conv1d support for torch

@fursund fursund added the feature request Functionality does not currently exist, would need to be created as a new feature (type) label Feb 3, 2023
@TobyRoseman
Copy link
Collaborator

Looking at the PyTorch ops we support, we are missing support for conv1d.

However the following works:

import coremltools as ct
import torch

m_torch = torch.nn.Conv1d(16, 33, 3)
x = torch.randn(20, 16, 50)
m_torch = torch.jit.trace(m_torch, x)

m_cm = ct.convert(m_torch, inputs=[ct.TensorType(shape=x.shape, name="x")])
m_cm.predict({'x': x})

The conv1d torch op must be getting lowered to a op we do support.

Can someone provide a toy example where our lack of conv1d support causes model conversion to fail?

@fursund
Copy link
Author

fursund commented Feb 4, 2023

With this https://gist.github.com/fursund/39c897d25f583686fe2626c56b48ffa3 and coremltools 6.2 it will hit the conv1d op

@fursund
Copy link
Author

fursund commented Feb 5, 2023

Best bet is that it has something to do with: https://asteroid.readthedocs.io/en/v0.3.1/_modules/asteroid/filterbanks/enc_dec.html#Encoder

@TobyRoseman
Copy link
Collaborator

With this https://gist.github.com/fursund/39c897d25f583686fe2626c56b48ffa3 and coremltools 6.2 it will hit the conv1d op

Did you mean to share a different link? The error here is not related to conv1d.

Best bet is that it has something to do with: https://asteroid.readthedocs.io/en/v0.3.1/_modules/asteroid/filterbanks/enc_dec.html#Encoder

There is quite a bit of code on this page. None of it is using coremltools. Can you provide a minimal example were conversion fails because we do not support conv1d?

@fursund
Copy link
Author

fursund commented Feb 7, 2023

Ok. Tried to reduce the issue a bit:

import coremltools as ct
import torch
from asteroid_filterbanks import Encoder, ParamSincFB

m_torch = Encoder(
                ParamSincFB(
                    80,
                    251,
                    stride=1,
                    sample_rate=16000,
                    min_low_hz=50,
                    min_band_hz=50,
                )
            )
print(m_torch)
x = torch.randn(10, 1, 1024)
m_torch = torch.jit.trace(m_torch, x)

m_cm = ct.convert(m_torch, inputs=[ct.TensorType(shape=x.shape, name="x")])

@TobyRoseman
Copy link
Collaborator

Thanks @fursund. After running pip install asteroid-filterbanks, I can reproduce the problem using your code.

Looks like there is still a lot going on inside of Encoder and ParamSincFb. Ideally, we'd have a toy example (i.e. something we could use as a unit test).

@fursund
Copy link
Author

fursund commented Feb 8, 2023

Yeah. Not involved in that project, but when I get a moment I'll try and make the test even more barebones.

@yych42
Copy link

yych42 commented Jun 6, 2023

Did anyone make progress on this? I was trying to convert titanet but failed with this issue.

@alealv alealv linked a pull request Oct 12, 2023 that will close this issue
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
feature request Functionality does not currently exist, would need to be created as a new feature (type)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants