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

Local tokenizer and processor for more consistent CI #16

Merged
merged 14 commits into from
Jun 10, 2024

Conversation

farzadab
Copy link
Contributor

@farzadab farzadab commented Jun 7, 2024

No description provided.

@farzadab farzadab marked this pull request as ready for review June 7, 2024 22:57
@farzadab farzadab requested a review from juberti June 7, 2024 23:02
@farzadab farzadab changed the title Local tokenizer and processor for faster CI Local tokenizer and processor for more consistent CI Jun 7, 2024
Copy link
Contributor

@juberti juberti left a comment

Choose a reason for hiding this comment

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

CI on this PR seems to work, so that's good!

ultravox/inference/infer_test.py Outdated Show resolved Hide resolved
ultravox/inference/infer_test.py Show resolved Hide resolved
@juberti
Copy link
Contributor

juberti commented Jun 9, 2024

Still seeing failures here even with local tokenizer/processor. I think something is getting wedged during test shutdown, kind of frustrating to debug since everything works fine locally 🙁

@juberti
Copy link
Contributor

juberti commented Jun 10, 2024

OK, so the issue here was ddp_utils_test.py, not infer_test.py. That said, I think this is worth keeping since it allows external PRs to access the Llama 3 tokenizer, which would otherwise fail due to not having the HF_TOKEN set.

@juberti juberti merged commit 8a3bc75 into main Jun 10, 2024
1 check passed
@farzadab farzadab deleted the farzad-tokenizer-test-local branch June 10, 2024 16:12
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.

2 participants