Skip to content

feat. Add text-to-video - Wan2.1-T2V-14B from Novita #1263

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 6 commits into from
Mar 17, 2025
Merged

feat. Add text-to-video - Wan2.1-T2V-14B from Novita #1263

merged 6 commits into from
Mar 17, 2025

Conversation

viktor2077
Copy link
Contributor

No description provided.

julien-c
julien-c reviewed Mar 10, 2025
View reviewed changes
Copy link
Member

@julien-c julien-c left a comment

Choose a reason for hiding this comment

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

@viktor2077 we changed filenames a bit last week, can you please rebase this PR on main? Thanks!

Wauplin
Wauplin reviewed Mar 10, 2025
View reviewed changes
Copy link
Contributor

@Wauplin Wauplin left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! I haven't tested myself yet (requires server-side update).
Also can you remove all the console.log before final review? Thanks!

@Wauplin Wauplin mentioned this pull request Mar 10, 2025
Merged
Wauplin added a commit that referenced this pull request Mar 10, 2025
@Wauplin
[Inference providers] Root-only base URLs (#1266)
bae2997 
Related to
huggingface-internal/moon-landing#12849 and
huggingface-internal/moon-landing#12855 (private
repo). Similar PR for Python client
huggingface/huggingface_hub#2918.

It is best if all base URLs for inference providers are
"root-domain-only" i.e. without subpaths. This PR updates Black Forest
Labs and Fireworks AI providers. Novita is taken care of in
#1263.

Note that this change requires a server-side update to make things
backward compatible (i.e. legacy clients using a subpath are still
working). This fix is already handled server-side.
@viktor2077 viktor2077 force-pushed the feat/novita-wan-t2v branch from f79f52f to 51f87e7 Compare March 11, 2025 13:09
@viktor2077
Copy link
Contributor Author

Thanks for the PR! I haven't tested myself yet (requires server-side update). Also can you remove all the console.log before final review? Thanks!

Thanks for the feedback ❤️ And I've removed all the console.log statements and fixed the conflicts.

@hanouticelina hanouticelina mentioned this pull request Mar 11, 2025
Merged
julien-c
julien-c approved these changes Mar 11, 2025
View reviewed changes
Copy link
Member

@julien-c julien-c left a comment

Choose a reason for hiding this comment

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

launching the CI, lgtm if green

Wauplin
Wauplin reviewed Mar 11, 2025
View reviewed changes
packages/inference/test/tapes.json Outdated
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure we want this test recorded in the repo as it would explode the tapes.json file quite quickly. @SBrandeis any idea how to deal with that?
(a solution can simply to not test it in CI)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @Wauplin , is there anything I can do in terms of this issue?

Copy link
Contributor

Choose a reason for hiding this comment

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

Hi @viktor2077 thanks for pinging me. We've discussed it privately and it'd be best to remove this tape. Could you remove the VCR test for text-to-video from the PR?

We are wary of making the repo history too big for quite little impact. Mid-term we are thinking of a way to make these tests more reliable, getting rid of the tapes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Got it, I've removed the VCR test changes. ✅

Wauplin
Wauplin reviewed Mar 11, 2025
View reviewed changes
Copy link
Contributor

@Wauplin Wauplin left a comment

Choose a reason for hiding this comment

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

Thanks! Left a comment to fix the CI + one remaining question about saving the video in tapes.json. Otherwise the logic looks good to me :)

@viktor2077 viktor2077 force-pushed the feat/novita-wan-t2v branch from b0a49ac to d72d036 Compare March 14, 2025 14:27
Wauplin
Wauplin reviewed Mar 14, 2025
View reviewed changes
Copy link
Contributor

@Wauplin Wauplin left a comment

Choose a reason for hiding this comment

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

Thanks! Could you also remove the test? We will add it back once we're decided on how we will handle these. In the meantime, no need to block this PR from being merged 🤗

@viktor2077 viktor2077 force-pushed the feat/novita-wan-t2v branch from a48e92e to b7d3b4f Compare March 14, 2025 15:05
Wauplin
Wauplin approved these changes Mar 14, 2025
View reviewed changes
Copy link
Contributor

@Wauplin Wauplin left a comment

Choose a reason for hiding this comment

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

All good! Thanks for iterating on it :) Will merge once the CI is green.

@viktor2077
Copy link
Contributor Author

viktor2077 commented Mar 17, 2025 •
edited
Loading

All good! Thanks for iterating on it :) Will merge once the CI is green.

Hi @Wauplin, it looks like we are ready 🚀

@Wauplin Wauplin merged commit 69ad498 into huggingface:main Mar 17, 2025
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@julien-c julien-c julien-c approved these changes

@Wauplin Wauplin Wauplin approved these changes

@hanouticelina hanouticelina Awaiting requested review from hanouticelina hanouticelina is a code owner

@SBrandeis SBrandeis Awaiting requested review from SBrandeis SBrandeis is a code owner

@coyotte508 coyotte508 Awaiting requested review from coyotte508

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@viktor2077 @julien-c @Wauplin