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

Overwritten zeroshot templates for webdataset eval #109

Closed
djghosh13 opened this issue Oct 13, 2023 · 2 comments
Closed

Overwritten zeroshot templates for webdataset eval #109

djghosh13 opened this issue Oct 13, 2023 · 2 comments

Comments

@djghosh13
Copy link
Contributor

#98 added better support for custom zeroshot prompt templates. However, the exact implementation results in behavior which conflicts with webdataset datasets (e.g. "wds/cifar10") stored on Huggingface or locally. Specifically:

  1. For zeroshot classification, templates are loaded from en_zeroshot_classification_templates.json
  2. If the dataset name is not recognized, the templates are set to IN1k templates
  3. build_wds_dataset loads classnames and templates from the source (local filesystem or Huggingface repo) (line 731)
  4. ds.templates = templates overwrites the templates on line 45

This affects all wds datasets, though it probably hasn't changed results in the majority of cases, where the dataset name and templates match the defaults in en_zeroshot_classification_templates.json.

A fix should probably either:

  • Never overwrite ds.templates if it already exists; or
  • Overwrite ds.templates only if the custom_template_file parameter is set (i.e. custom template takes precedence over webdataset template)
@mehdidc
Copy link
Collaborator

mehdidc commented Oct 14, 2023

Thanks a lot @djghosh13 ! I haven't noticed this, I think it would make sense to overwrite only if custom option is provided (second option). Will do a PR mentioning the issue in datacomp as well. So basically order of precedence would be:

1 - templates from --custom_template_file (if provided).
2 - the dataset existing templates (if it has, like in WDS case, or Babel ImageNet)
3 - templates from <LANG>_zeroshot_classification_templates.json if dataset name is in there, otherwise imagenet1k templates.

The other thing I am thinking of, is to (optionally) write the classnames/templates that ended up beging used in the JSON dump to have full transparency.

mehdidc added a commit that referenced this issue Dec 1, 2023
* fix overwritten zeroshot templates issue #109. Thanks to @djghosh13.

* handle non classification/linear probe case

* comments

* simplify

* support dumping the classnames and templates that are used for evaluation

* fix tests

* just comments

* just comments
@mehdidc
Copy link
Collaborator

mehdidc commented Dec 2, 2023

Fixed.

@mehdidc mehdidc closed this as completed Dec 2, 2023
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

No branches or pull requests

2 participants