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

Fix test fetcher (doctest) + Idefics2's doc example #30274

Merged
merged 1 commit into from
Apr 16, 2024
Merged

Conversation

ydshieh
Copy link
Collaborator

@ydshieh ydshieh commented Apr 16, 2024

What does this PR do?

  • test fetcher (doctest): a mistake I made in the previous PR
  • Idefics2's doc example: not able to fit into GPU even with fp16. So I changed it.

@ydshieh ydshieh requested a review from amyeroberts April 16, 2024 15:39
@@ -507,7 +507,7 @@ def get_all_doctest_files() -> List[str]:
# change to use "/" as path separator
test_files_to_run = ["/".join(Path(x).parts) for x in test_files_to_run]
# don't run doctest for files in `src/transformers/models/deprecated`
test_files_to_run = [x for x in test_files_to_run if "models/deprecated" not in test_files_to_run]
test_files_to_run = [x for x in test_files_to_run if "models/deprecated" not in x]
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

my bad


>>> # Generate
>>> generated_ids = model.generate(**inputs, bad_words_ids=BAD_WORDS_IDS, max_new_tokens=500)
>>> generated_ids = model.generate(**inputs, bad_words_ids=BAD_WORDS_IDS, max_new_tokens=20)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

it's very slow, so let's just use 20

>>> generated_texts = processor.batch_decode(generated_ids, skip_special_tokens=True)

>>> print(generated_texts)
['In this image, we can see the city of New York, and more specifically the Statue of Liberty. In this image, we can see the city of New York, and more specifically the Statue of Liberty.\n\n', 'In which city is that bridge located?\n\nThe bridge is located in the city of Pittsburgh, Pennsylvania.\n\n\nThe bridge is']
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The results are not very good. But we don't have it previously, so I don't know if we consider it bad.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm surprised by these generations - they also don't look like the outputs when I was integrating the model 🤔

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

maybe you were using other type of GPUs. CI is using T4.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Quite possibly - I was using NVIDIA A10G. I find it quite concerning the generations can be so much worse between hardware though, especially given this isn't a tiny model

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I can check it tomorrow. But I will merge first 🙏

@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
Collaborator

@amyeroberts amyeroberts 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 handling this!

>>> generated_texts = processor.batch_decode(generated_ids, skip_special_tokens=True)

>>> print(generated_texts)
['In this image, we can see the city of New York, and more specifically the Statue of Liberty. In this image, we can see the city of New York, and more specifically the Statue of Liberty.\n\n', 'In which city is that bridge located?\n\nThe bridge is located in the city of Pittsburgh, Pennsylvania.\n\n\nThe bridge is']
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm surprised by these generations - they also don't look like the outputs when I was integrating the model 🤔

@ydshieh ydshieh self-assigned this Apr 16, 2024
@ydshieh ydshieh merged commit 5fabebd into main Apr 16, 2024
8 checks passed
@ydshieh ydshieh deleted the fix_stupid_cond_pr branch April 16, 2024 19:25
zucchini-nlp pushed a commit to zucchini-nlp/transformers that referenced this pull request Apr 18, 2024
ArthurZucker pushed a commit that referenced this pull request Apr 22, 2024
ydshieh added a commit that referenced this pull request Apr 23, 2024
itazap pushed a commit that referenced this pull request May 14, 2024
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.

3 participants