Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Add support for torch op conv1d #1753

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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in 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