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

Make EncodecModel.decode ONNX exportable #29913

Merged
merged 4 commits into from
Apr 3, 2024

Conversation

fxmarty
Copy link
Contributor

@fxmarty fxmarty commented Mar 27, 2024

As per title. This is needed for the ONNX export of Musicgen e.g. for transformers.js

This removes an important warning:

/home/fxmarty/hf_internship/transformers/src/transformers/models/encodec/modeling_encodec.py:121: TracerWarning: Converting a tensor to a Python float might cause the trace to be incorrect. We can't record the data flow of Python values, so this value will be treated as a constant in the future. This means that the trace might not generalize to other inputs!
  ideal_length = (math.ceil(n_frames) - 1) * stride + (kernel_size - padding_total)

where a padding length was previously hard-coded and applied here

extra_padding = self._get_extra_padding_for_conv1d(hidden_states, kernel_size, stride, padding_total)
if self.causal:
# Left padding for causal
hidden_states = self._pad1d(hidden_states, (padding_total, extra_padding), mode=self.pad_mode)
else:
# Asymmetric padding required for odd strides
padding_right = padding_total // 2
padding_left = padding_total - padding_right
hidden_states = self._pad1d(
hidden_states, (padding_left, padding_right + extra_padding), mode=self.pad_mode
)

Comment on lines 128 to 129
n_frames = (length - self.kernel_size + self.padding_total) / self.stride + 1
ideal_length = ((torch.ceil(n_frames).to(torch.int64) - 1) * self.stride + (self.kernel_size - self.padding_total))
Copy link
Contributor Author

@fxmarty fxmarty Mar 27, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Essentially, we need these ops to be on tensors and not on python types (hence the registration of buffers).

The .to(torch.int64) is added because the produced ONNX model is wrong otherwise (try to concat the float ideal_length - length with padding_total, which is illegal

@HuggingFaceDocBuilderDev

The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.

Copy link
Contributor

@xenova xenova left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can confirm this fixes the issue with the ONNX export (tested with transformers.js)!

Copy link
Contributor

@ylacombe ylacombe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@fxmarty fxmarty merged commit 81642d2 into huggingface:main Apr 3, 2024
18 checks passed
Copy link
Collaborator

@ArthurZucker ArthurZucker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants