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 docs on zeroshot image classification prompt templates #31343

Merged
merged 6 commits into from
Jun 19, 2024

Conversation

aliencaocao
Copy link
Contributor

@aliencaocao aliencaocao commented Jun 10, 2024

What does this PR do?

Adds documentation on prompt templates for Zero Shot Image Classification tasks so that the output results match pipelines and are also correct. Not applying the template can lead to greatly worsened accuracy.

Fixes #30951

Before submitting

  • This PR fixes a typo or improves the docs (you can dismiss the other checks if that's the case).
  • Did you read the contributor guideline,
    Pull Request section?
  • Was this discussed/approved via a Github issue or the forum? Please add a link
    to it if that's the case.
  • Did you make sure to update the documentation with your changes? Here are the
    documentation guidelines, and
    here are tips on formatting docstrings.
  • Did you write any new necessary tests?

Who can review?

@amyeroberts

@aliencaocao
Copy link
Contributor Author

i have a PR that adds support for siglip training coming up so would be great if this could be merged asap so the docs don't merge conflict

@amyeroberts
Copy link
Collaborator

i have a PR that adds support for siglip training coming up so would be great if this could be merged asap so the docs don't merge conflict

@aliencaocao I appreciate that you want to have PRs merged in as soon as possible (we do too!) but managing conflicts for other PRs isn't something that we will consider, unless it's also for transformers something we want to merge in and linked to.

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 contributing to improving the docs!

I'm important to distinguish between what the user must feed to the model, and a useful example. The "This is a photo of {}" prompt just helps replicate the values from the pipeline, but it's not a requirement and can in fact be changed in the pipeline too.

docs/source/en/model_doc/siglip.md Outdated Show resolved Hide resolved
docs/source/en/model_doc/siglip.md Outdated Show resolved Hide resolved
docs/source/en/model_doc/siglip.md Outdated Show resolved Hide resolved
docs/source/en/tasks/zero_shot_image_classification.md Outdated Show resolved Hide resolved
Update usage tips
@aliencaocao
Copy link
Contributor Author

@amyeroberts heads up i've made the changes already

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!

Just two tiny nits

docs/source/en/tasks/zero_shot_image_classification.md Outdated Show resolved Hide resolved
docs/source/en/model_doc/siglip.md Outdated Show resolved Hide resolved
aliencaocao and others added 2 commits June 18, 2024 19:03
Co-authored-by: amyeroberts <22614925+amyeroberts@users.noreply.github.com>
Co-authored-by: amyeroberts <22614925+amyeroberts@users.noreply.github.com>
@aliencaocao
Copy link
Contributor Author

Merged your suggestions

@amyeroberts
Copy link
Collaborator

@aliencaocao Can you try rebasing to include the latest changes upstream? This should resolve some of the issues on the CI runs

@aliencaocao
Copy link
Contributor Author

@amyeroberts hi somehow the workflows arent getting triggered?

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

@amyeroberts
Copy link
Collaborator

@aliencaocao We've had a few issues with tests triggering on circle CI today. I'm not 100% sure of the cause, but other PRs which have blocked workflows are now working. Could you try triggering a new run? git commit --allow-empty -m "Trigger CI"

@aliencaocao
Copy link
Contributor Author

@amyeroberts its ready now

@amyeroberts
Copy link
Collaborator

Thanks for updating the docs and your patience with the CI!

@amyeroberts amyeroberts merged commit cd5f7c1 into huggingface:main Jun 19, 2024
8 checks passed
@aliencaocao aliencaocao deleted the zeroshot-img-cls-docs branch June 19, 2024 10:59
itazap pushed a commit that referenced this pull request Jun 20, 2024
* Add docs on pipeline templates

* Fix example and comments
Update usage tips

* Update docs/source/en/tasks/zero_shot_image_classification.md

Co-authored-by: amyeroberts <22614925+amyeroberts@users.noreply.github.com>

* Update docs/source/en/model_doc/siglip.md

Co-authored-by: amyeroberts <22614925+amyeroberts@users.noreply.github.com>

* Trigger CI

---------

Co-authored-by: amyeroberts <22614925+amyeroberts@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

google/siglip-so400m-patch14-384 inference output mismatch with pipeline output
3 participants