Skip to content

Comments

add doctests to TF ViT#16462

Closed
johko wants to merge 56 commits intohuggingface:mainfrom
johko:doc-test-vit-tf
Closed

add doctests to TF ViT#16462
johko wants to merge 56 commits intohuggingface:mainfrom
johko:doc-test-vit-tf

Conversation

@johko
Copy link
Contributor

@johko johko commented Mar 28, 2022

What does this PR do?

Add doctests for the TF version of ViT

Fixes # (issue)
#16292

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?

@patrickvonplaten

Returns:

Examples:

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wasn't sure if there is supposed to be an empty line here or not, for me it looked better without it

Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's just keep the empty line. It is done this way in other places.

>>> last_hidden_states = outputs.last_hidden_state
>>> list(last_hidden_states.shape)
[1, 197, 768]
```"""
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also wasn't sure about the style here, keep ``` and """ in the same line or split to two lines

Copy link
Collaborator

Choose a reason for hiding this comment

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

It should be more or less a style choice. In the model files, we usually keep it as ```"""

@HuggingFaceDocBuilderDev
Copy link

HuggingFaceDocBuilderDev commented Mar 28, 2022

The documentation is not available anymore as the PR was closed or merged.

@ydshieh
Copy link
Collaborator

ydshieh commented Mar 28, 2022

Hi, @johko

Thank you for this PR! Really appreciated.

I just realized that we have PT_VISION_BASE_MODEL_SAMPLE, but not TF_VISION_BASE_MODEL_SAMPLE.

Ideally, we would like to re-use code sample in doc.py. I will need to discuss with the team to make a decision.

I will come back to you once we have a decision. Sorry for the inconvenience!

@ydshieh
Copy link
Collaborator

ydshieh commented Mar 29, 2022

Hi, @johko

After the discussion with the team, we think it would be really better for us to add the following in doc.py

TF_VISION_BASE_MODEL_SAMPLE
TF_VISION_SEQ_CLASS_SAMPLE

(as already done in PyTorch side).

I will open a PR to add these, and keep you updated when that PR is merged. Thank you.

arnaudstiegler and others added 10 commits March 29, 2022 16:19
* docstring still WIP | adding to documentation_tests

* clean version | passes tests

* adding to documentation_test

* adding forward for training pass

* make fixup applied

* address comments

* fix doctest

* apply make fixup

* remove additional blank

* fix file to have correct split for prepare_for_doc_test

* Update src/transformers/models/trocr/modeling_trocr.py

Co-authored-by: Yih-Dar <2521628+ydshieh@users.noreply.github.com>

* address comments

* changing text | adding loss check | make fixup

* make fixup

* Update src/transformers/models/trocr/modeling_trocr.py

Co-authored-by: Yih-Dar <2521628+ydshieh@users.noreply.github.com>

* Update src/transformers/models/trocr/modeling_trocr.py

Co-authored-by: Yih-Dar <2521628+ydshieh@users.noreply.github.com>

* Update src/transformers/models/trocr/modeling_trocr.py

Co-authored-by: Yih-Dar <2521628+ydshieh@users.noreply.github.com>

* make fixup

Co-authored-by: Yih-Dar <2521628+ydshieh@users.noreply.github.com>
…uggingface#16475)

* Prevent overwriting matched with mismatched metrics

* Fix style
* Remove duplicate mLuke

* 🖍 apply feedback
…e#16271)

* fix - set output_attentions to True

* Update tests/test_modeling_flax_common.py

* update for has_attentions

* overwrite check_outputs in FlaxBigBirdModelTest

Co-authored-by: ydshieh <ydshieh@users.noreply.github.com>
Co-authored-by: Suraj Patil <surajp815@gmail.com>
* Fix for test_mixed_precision

* Fix test_saved_model_creation by using shape_list instead of shape

* skit test_model_from_pretrained on GPU for now to avoid GPU OOM

* skip test_gptj_sample_max_time for now

Co-authored-by: ydshieh <ydshieh@users.noreply.github.com>
* add code samples

Co-authored-by: ydshieh <ydshieh@users.noreply.github.com>
* Add type hints for UniSpeech

* Added type hints for UniSpeechSat

* Added type hints for Wave2Vec2 (PT)

* Added type hints for models dependent of wave2vec
@ydshieh
Copy link
Collaborator

ydshieh commented Mar 29, 2022

@johko

TF_VISION_BASE_MODEL_SAMPLE
TF_VISION_SEQ_CLASS_SAMPLE

are added in this PR (and already merged to main) #16477

Once you pull the upstream's main in your local clone's main (or master), and rebase or merge it into your working branch, the process is just a matter of using @add_code_sample_docstrings together with expected_output, checkpoint, etc. You can take this as a reference:

https://github.com/huggingface/transformers/pull/16363/files#diff-5707805d290617078f996faf1138de197fa813f78c0aa5ea497e73b5228f1103

Regarding the checkpoint to use:

  • TFViTModel
    "google/vit-base-patch16-224-in21k"

  • TFViTForImageClassification
    "google/vit-base-patch16-224"

I manually checked the code sample. Let me know if you encounter any issue.

gante and others added 5 commits March 29, 2022 18:17
…ace#16465)

* properly handle kwargs in encoder_decoder architectures

* make fixup
* ported TFViTMAEIntermediate and TFViTMAEOutput.

* added TFViTMAEModel and TFViTMAEDecoder.

* feat: added a noise argument in the implementation for reproducibility.

* feat: vit mae models with an additional noise argument for reproducibility.

Co-authored-by: ariG23498 <aritra.born2fly@gmail.com>
Co-authored-by: ydshieh <ydshieh@users.noreply.github.com>
* Avoid accessing .dataset of a dataloader

* style

* fix

* cleaning up, reverting some misunderstandings

* black

* add train_dataset argument to get_train_dataloader, and fix other instances of length checks

* flake8

* address comments

* fix bug

* cleanup

* add test

* Update tests/trainer/test_trainer.py

Co-authored-by: Sylvain Gugger <35901082+sgugger@users.noreply.github.com>

* under torch

* merge

* stylistic suggestion

Co-authored-by: Sander Land <sander@chatdesk.com>
Co-authored-by: Sylvain Gugger <35901082+sgugger@users.noreply.github.com>
…ce#16311)

* add unpack_inputs decorator to Main Layer

* add unpack_inputs decorator to Model

* add unpack_inputs decorator to LMHead Model

* add unpack_inputs decorator to Double Head Model

* add unpack_inputs decorator to Sequence Classification Model

* run fixup recipe

* make unpack_inputs the first decorator
@johko
Copy link
Contributor Author

johko commented Mar 29, 2022

@ydshieh
Thanks for the explanation, I'll look into it and implement the changes within the next days

ydshieh and others added 4 commits March 29, 2022 22:12
* Raise diff tolerance value

Co-authored-by: ydshieh <ydshieh@users.noreply.github.com>
… initailized (huggingface#16487)

* Do not initialize torch process group twice

* Apply suggestions from code review
* Type hints and TF decorator added

* Type hints and TF decorator added

* make style

Co-authored-by: matt <rocketknight1@gmail.com>
SimplyJuanjo and others added 23 commits March 31, 2022 07:43
* Duplication of the source eng file

* Spanish translation of the file multilingual.mdx

* Update docs/source_es/multilingual.mdx

Co-authored-by: Omar U. Espejel <espejelomar@gmail.com>

* Update docs/source_es/multilingual.mdx

Co-authored-by: Omar U. Espejel <espejelomar@gmail.com>

* Update docs/source_es/multilingual.mdx

Co-authored-by: Omar U. Espejel <espejelomar@gmail.com>

* Update docs/source_es/multilingual.mdx

Co-authored-by: Omar U. Espejel <espejelomar@gmail.com>

* Update docs/source_es/multilingual.mdx

Co-authored-by: Omar U. Espejel <espejelomar@gmail.com>

* Update docs/source_es/multilingual.mdx

Co-authored-by: Omar U. Espejel <espejelomar@gmail.com>

* Update docs/source_es/multilingual.mdx

Co-authored-by: Omar U. Espejel <espejelomar@gmail.com>

* Fix nits and finish translation

Co-authored-by: Omar U. Espejel <espejelomar@gmail.com>
* Translate installation.mdx to Spanish

* Update docs/source_es/installation.mdx

Co-authored-by: Omar U. Espejel <espejelomar@gmail.com>

* Update docs/source_es/installation.mdx

Co-authored-by: Omar U. Espejel <espejelomar@gmail.com>

* Update docs/source_es/installation.mdx

Co-authored-by: Omar U. Espejel <espejelomar@gmail.com>

* Update docs/source_es/installation.mdx

Co-authored-by: Omar U. Espejel <espejelomar@gmail.com>

* Update docs/source_es/installation.mdx

Co-authored-by: Omar U. Espejel <espejelomar@gmail.com>

* Update docs/source_es/installation.mdx

Co-authored-by: Omar U. Espejel <espejelomar@gmail.com>

* Update docs/source_es/installation.mdx

Co-authored-by: Omar U. Espejel <espejelomar@gmail.com>

* Update docs/source_es/installation.mdx

Co-authored-by: Omar U. Espejel <espejelomar@gmail.com>

* Update docs/source_es/installation.mdx

Co-authored-by: Omar U. Espejel <espejelomar@gmail.com>

* Update docs/source_es/installation.mdx

Co-authored-by: Omar U. Espejel <espejelomar@gmail.com>

* Fix nits and finish translation

Co-authored-by: Omar U. Espejel <espejelomar@gmail.com>
* Translate accelerate.mdx from english to spanish

* Update docs/source_es/accelerate.mdx

Co-authored-by: Omar U. Espejel <espejelomar@gmail.com>

* Apply suggestions from code review

Co-authored-by: Omar U. Espejel <espejelomar@gmail.com>

* Apply suggestions from code review

Co-authored-by: Omar U. Espejel <espejelomar@gmail.com>

* Fix nits and finish translation

Co-authored-by: Omar U. Espejel <espejelomar@gmail.com>
* added type hints to xglm pytorch

* Update src/transformers/models/xglm/modeling_xglm.py

* Update src/transformers/models/xglm/modeling_xglm.py

Co-authored-by: Matt <Rocketknight1@users.noreply.github.com>
* [research] link to the XTREME-S paper

* Update examples/research_projects/xtreme-s/README.md

Co-authored-by: Lysandre Debut <lysandre.debut@reseau.eseo.fr>

Co-authored-by: Lysandre Debut <lysandre.debut@reseau.eseo.fr>
* Add beit onnx conversion support

* Updated docs

* Added cross reference to ViT ONNX config
* Add type hints to PLBart PyTorch

* Remove pending merge conflicts

* Fix PLBart Type Hints

* Add changes from review
* added type hints for mbart tensorflow tf implementation

* Adding missing type hints for mBART model 

Tensorflow Implementation model added with missing type hints

* Missing Type hints - correction

For TF model

* Code fixup using make quality tests

* Hint types - typo error

* make fix-copies and make fixup

* type hints

* updated files

* type hints update

* making dependent modesls coherent

Co-authored-by: matt <rocketknight1@gmail.com>
)

* Remove MBart subclass of XLMRoberta in tokenzier

* Fix style

* Copy docs from MBart50 tokenizer
* use random_attention_mask for TF tests

* Fix for TFCLIP test (for now).

Co-authored-by: ydshieh <ydshieh@users.noreply.github.com>
Co-authored-by: Niels Rogge <nielsrogge@nielss-mbp.home>
* Pin tokenizers version <0.13

* Style
Co-authored-by: ydshieh <ydshieh@users.noreply.github.com>
@johko
Copy link
Contributor Author

johko commented Apr 1, 2022

@ydshieh
Ah, sorry I messed up the history now, I'm still getting used to the VS Code Git plugin.
But I added the add_code_sample_docstrings decorator to modeling_tf_vit.py now

@ydshieh
Copy link
Collaborator

ydshieh commented Apr 4, 2022

@johko

Thank you for the update!

Regarding the commit history, could you try to fix it. I never need to deal with this situation so far, but here are a few threads I could find (potentially) relevant:

Please note that these links are merely an attempt to fix the current situation, I could not guarantee that the mentioned methods would work well and without any problem.

In any case, since the real change is very small, you can always update the master/main branch of your local clone, and create a new branch + add the changes there, then open a new PR.

Thank you for your understanding!

@johko
Copy link
Contributor Author

johko commented Apr 4, 2022

Thank you, I'll look into it and try to fix it. In the end, as you mentioned I might always just make a new branch from my local master

@johko
Copy link
Contributor Author

johko commented Apr 4, 2022

@ydshieh I'll close this PR and create a new one with a clean history. It seems VS Code used git sync and this messes up the history after a rebase

@johko johko closed this Apr 4, 2022
@ydshieh
Copy link
Collaborator

ydshieh commented Apr 4, 2022

No problem, @johko . Thank you for the effort!

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.