[DocTest] Doctest for Electra Pytorch#16628

Closed

bhadreshpsavani wants to merge 27 commits intohuggingface:mainfrom

bhadreshpsavani:add-doctests-electra

Contributor

bhadreshpsavani commented Apr 6, 2022 •

edited

Loading

What does this PR do?

Adds Doctest fo Electra Pytorch

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?

bhadreshpsavani added 2 commits

April 6, 2022 17:10


          Doctest for Pytorch

b34eb19


          Merge branch 'main' into add-doctests-electra

509ca47

HuggingFaceDocBuilderDev commented Apr 6, 2022 •

edited

Loading

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

ydshieh self-requested a review

April 6, 2022 12:20

ydshieh reviewed

View reviewed changes

Collaborator

ydshieh left a comment

Hi, @bhadreshpsavani

It looks great, thank you for your work! I leave a few comments.
(I also saw you uploaded a checkpoint, very nice!)

src/transformers/models/electra/modeling_electra.py Outdated

                       config_class=_CONFIG_FOR_DOC,
+                      mask="[mask]",
+                      expected_output="'togo'",
+                      expected_loss=11.43,

Collaborator

ydshieh Apr 6, 2022

To use Electra for mask LM task, we need checkpoints like

https://huggingface.co/google/electra-small-generator

You can find more with

https://huggingface.co/models?pipeline_tag=fill-mask&sort=downloads&search=electra

The

_CHECKPOINT_FOR_DOC = "google/electra-small-discriminator"

is not good for Mask LM, since it is

ELECTRA models are trained to distinguish "real" input tokens vs "fake" input tokens
generated by another neural network, similar to the discriminator of a [GAN]
(https://arxiv.org/pdf/1406.2661.pdf).

See https://huggingface.co/google/electra-small-discriminator

src/transformers/models/electra/modeling_electra.py

                       ...     0
                       >>> )  # Batch size 1
                       >>> logits = model(input_ids).logits

Collaborator

ydshieh Apr 6, 2022

It would be a good idea to use the example shown here

https://huggingface.co/google/electra-small-discriminator

src/transformers/models/electra/modeling_electra.py Outdated


		>>> prediction_logits = outputs.logits

Collaborator

ydshieh Apr 6, 2022

You can remove these extra new lines :-)

src/transformers/models/electra/modeling_electra.py

                   @add_code_sample_docstrings(
                       processor_class=_TOKENIZER_FOR_DOC,
-                      checkpoint=_CHECKPOINT_FOR_DOC,
+                      checkpoint="deepset/electra-base-squad2",

Collaborator

ydshieh Apr 6, 2022

nice!

src/transformers/models/electra/modeling_electra.py Outdated

                   @add_code_sample_docstrings(
                       processor_class=_TOKENIZER_FOR_DOC,
-                      checkpoint=_CHECKPOINT_FOR_DOC,
+                      checkpoint="bhadresh-savani/electra-base-discriminator-finetuned-conll03-english ",

Collaborator

ydshieh Apr 6, 2022

nice!

bhadreshpsavani and others added 6 commits

April 6, 2022 22:49


          upaded model file

ce8716b


          fixes additional space in model name

c475e5e


          Empty commit

bd3ab18


          Merge branch 'huggingface:main' into add-doctests-electra

e6352b2


          Empty commit

cca3a14


          Fix doc example (huggingface#16448)

dc99180

* Fix doc

* Make fixup

Co-authored-by: Niels Rogge <nielsrogge@nielss-mbp.home>

Contributor Author

bhadreshpsavani commented Apr 7, 2022 •

edited

Loading

Hi @ydshieh,
The issue we discussed in Issue:16292
For PyTorch its running fine,
only In case of TensorFlow its giving issue,
I am sharing notebook for reproducing the issue.

Collaborator

ydshieh commented Apr 7, 2022

Hi @ydshieh, The issue we discussed in Issue:16292 For PyTorch its running fine, only In case of TensorFlow its giving issue, I am sharing notebook for reproducing the issue.

Hi, I am not able to open it. Could you make it public?

Contributor Author

bhadreshpsavani commented Apr 7, 2022

Hi @ydshieh,
I have updated the link

Collaborator

ydshieh commented Apr 7, 2022 •

edited

Loading

@bhadreshpsavani Thanks. It looks like PyTorch somehow made it to run, despite the indices are wrong and out of range.
It gave strange loss value though.

I opened a PR to address this situation

#16648. Let's wait it being reviewed and merged to proceed.

Thank you for your patience!

lmvasque and others added 15 commits

April 7, 2022 10:02


          Add inputs vector to calculate metric method (huggingface#16461)

09a272b

* Add inputs vector to calculate metric method

* Include inputs for evaluation metrics with backwards compatibility

* Prevent inputs create OOM issue and documentation details

* Update style and code documentation

* Fix style formatting issues

* Update files format with make style


          [megatron-bert-uncased-345m] fix conversion (huggingface#16639)

080e42d


          Remove parent/child tests in auto model tests (huggingface#16653)

389f661


          Updated _load_pretrained_model_low_mem to check if keys are in the st…

…ate_dict (huggingface#16643)

* Updated _load_pretrained_model_low_mem to check if keys are in the stored state_dict

* update after conversions


          Update Support image on README.md (huggingface#16615)

3e26e78

* Update README.md Support Image

Updates the Support image linking to our EAP page (to give it a refresh + help avoid image fatigue).

Slack thread checking in with #open-source-internal on this update (https://huggingface.slack.com/archives/C021H1P1HKR/p1648838903316709)

* Compressed Updated Support image

* Improves Support Image Logo + Height

Updated the image based on logo + size feedback. Big thanks to Bibi for making quick edits to this image.


          RegNet (huggingface#16188)

af14c61

* base model done

* make style

* done

* added files

* Apply suggestions from code review

Co-authored-by: NielsRogge <48327001+NielsRogge@users.noreply.github.com>

* Apply suggestions from code review

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

* Trigger doc build

* resolved conversations

* resolved conversations

* seer models

* minor changes

* minor changes

* make fixup

* glob variables

* minor changes

* fix copies

* config when possibile

* resolved conflicts

* resolved conflicts

* resolved conflicts

* CI

* conversion script for 10b param

* fixed for 10b model

* minor updates in the doc + make style

* removed unused code

* Apply suggestions from code review

Co-authored-by: NielsRogge <48327001+NielsRogge@users.noreply.github.com>

* removed unused code

* removed unused code

* updated modeling_utils from main

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


          bert: properly mention deprecation of TF2 conversion script (huggingf…

33cb211

…ace#16171)


          Add TAPEX (huggingface#16473)

4ef0abb

* Add TapexTokenizer

* Improve docstrings and provide option to provide answer

* Remove option for pretokenized inputs

* Add TAPEX to README

* Fix copies

* Remove option for pretokenized inputs

* Initial commit: add tapex fine-tuning examples on both table-based question answering and table-based fact verification.

* - Draft a README file for running the script and introducing some background.
- Remove unused code lines in tabfact script.
- Disable the deafult `pad_to_max_length` option which is memory-consuming.

* * Support `as_target_tokenizer` function for TapexTokenizer.
* Fix the do_lower_case behaviour of TapexTokenizer.
* Add unit tests for target scenarios and cased/uncased scenarios for both source and target.

* * Replace the label BartTokenizer with TapexTokenizer's as_target_tokenizer function.
* Fix typos in tapex example README.

* * fix the evaluation script - remove the property `task_name`

* * Make the label space more clear for tabfact tasks

* * Using a new fine-tuning script for tapex-base on tabfact.

* * Remove the lowercase code outside the tokenizer - we use the tokenizer to control whether do_lower_case
* Guarantee the hyper-parameter can be run without out-of-memory on 16GB card and report the new reproduced number on wikisql

* * Remove the default tokenizer_name option.
* Provide evaluation command.

* * Support for WikiTableQuestion dataset.

* Fix a typo in README.

* * Fix the datasets's key name in WikiTableQuestions

* Run make fixup and move test to folder

* Fix quality

* Apply suggestions from code review

* Apply suggestions from code review

Co-authored-by: Suraj Patil <surajp815@gmail.com>

* Apply suggestions from code review

* Apply suggestions from code review

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

* Apply some more suggestions from code review

* Improve docstrings

* Overwrite failing test

* Improve comment in example scripts

* Fix rebase

* Add TAPEX to Auto mapping

* Add TAPEX to auto config mappings

* Put TAPEX higher than BART in auto mapping

* Add TAPEX to doc tests

Co-authored-by: Niels Rogge <nielsrogge@Nielss-MBP.localdomain>
Co-authored-by: SivilTaram <qianlxc@outlook.com>
Co-authored-by: Niels Rogge <nielsrogge@nielss-mbp.home>
Co-authored-by: Suraj Patil <surajp815@gmail.com>
Co-authored-by: Sylvain Gugger <35901082+sgugger@users.noreply.github.com>
Co-authored-by: Niels Rogge <nielsrogge@Nielss-MacBook-Pro.local>


          add vit tf doctest with @add_code_sample_docstrings (huggingface#16636)

9db2eeb

* add vit tf doctest with @add_code_sample_docstrings

* add labels string back in

Co-authored-by: Johannes Kolbe <johannes.kolbe@tech.better.team>


          Fix error in doc of DataCollatorWithPadding (huggingface#16662)

5db2fcc

The defalut value of `padding` in `DataCollatorWithPadding` is `True`, not `False`.


          Fix style

9a24b97


          Fix QA sample (huggingface#16648)

ab22966

* fix QA sample

* For TF_QUESTION_ANSWERING_SAMPLE

Co-authored-by: ydshieh <ydshieh@users.noreply.github.com>


           Add tests for no_trainer and fix existing examples (huggingface#16656)

d57da99

* Fixed some bugs involving saving during epochs
* Added tests mimicking the existing examples tests
* Added in json exporting to all `no_trainer` examples for consistency


          Doctest for Pytorch

50f7da3


          upaded model file

de66e69

bhadreshpsavani added 4 commits

April 8, 2022 20:12


          fixes additional space in model name

de5638f


          Empty commit

cadbb8c


          Empty commit

d1ce7dd


          Merge branch 'add-doctests-electra' of https://github.com/bhadreshpsa…

454f586

…vani/transformers into add-doctests-electra

Contributor Author

bhadreshpsavani commented Apr 8, 2022 •

edited

Loading

When I used this code and rebase at the end I messed up the commits, is there any way to remove earlier commits from this PR?

I think I need to close this PR and create a separate one

Collaborator

ydshieh commented Apr 8, 2022 •

edited

Loading

Hi, @bhadreshpsavani

I think close this PR and create a new one is the easiest way (considering the change in your PR is not so large).
Thank you for the effort.

BTW, I think you used git merge rather than git rebase. In general, git rebase could avoid this situation :-).

(There might be other ways of doing things, but I am not a real git expert)

Contributor Author

bhadreshpsavani commented Apr 8, 2022

I followed this,

git checkout main  # or `master`, depends on your local clone
git fetch upstream
git pull upstream main  # Hugging Face `transformers` renamed the default branch to `main` recently
git checkout your_working_branch_for_this_sprint
git rebase main  # or `master`

In the end, there was a merge conflict so i use something like git rebase --continue after solving the conflict and sync the changes. It got messed up!

I need to be more careful!

bhadreshpsavani closed this

bhadreshpsavani deleted the add-doctests-electra branch

April 8, 2022 14:57

Collaborator

ydshieh commented Apr 8, 2022

I followed this,
git checkout main  # or `master`, depends on your local clone
git fetch upstream
git pull upstream main  # Hugging Face `transformers` renamed the default branch to `main` recently
git checkout your_working_branch_for_this_sprint
git rebase main  # or `master`
In the end, there was a merge conflict so i use something like git rebase --continue after solving the conflict and sync the changes. It got messed up!

I need to be more careful!

OK, conflict is not easy thing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet