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

[WIP] Test TF Flaubert + Add {XLM, Flaubert}{TokenClassification, MultipleC… #5614

Merged
merged 8 commits into from
Jul 29, 2020

Conversation

LysandreJik
Copy link
Member

@LysandreJik LysandreJik commented Jul 9, 2020

…hoice} models and tests

The remaining TF tests pass with TF2.3. Waiting to unpin TF before merge.

Copy link
Contributor

@jplu jplu left a comment

Choose a reason for hiding this comment

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

Can you try to change line 261:

dropout_probability = random.uniform(0, 1)

Into

dropout_probability = tf.random.uniform([], 0, 1)

To see if it changes something?

@@ -136,8 +139,8 @@ def call(
head_mask=None,
inputs_embeds=None,
training=False,
Copy link
Contributor

Choose a reason for hiding this comment

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

I would suggest to put training at the end.

attn = attn_outputs[0]
attentions = attentions + (attn_outputs[1],)
if output_attentions:
Copy link
Contributor

Choose a reason for hiding this comment

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

Replace by:

if cast_bool_to_primitive(output_attentions, self.output_attentions) is True:

Same at lines 285 and 321.

Replace as well:

if output_hidden_states:

By

if cast_bool_to_primitive(output_hidden_states, self.output_hidden_states) is True:

At lines 265, 308, and 319

Copy link
Collaborator

@sgugger sgugger left a comment

Choose a reason for hiding this comment

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

A few nits to clean up ut looks great otherwise!

@@ -140,6 +140,7 @@ def test_attention_outputs(self):
encoder_seq_length = encoder_seq_length * self.model_tester.num_hashes

for model_class in self.all_model_classes:
print(model_class)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Left from debugging?

Copy link
Member Author

Choose a reason for hiding this comment

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

oops

tests/test_modeling_flaubert.py Outdated Show resolved Hide resolved
class TFFlaubertModelTest(TFModelTesterMixin, unittest.TestCase):

all_model_classes = (
# TODO The multiple choice model is missing and should be added.
Copy link
Collaborator

Choose a reason for hiding this comment

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

The TODO seems outdated.

@codecov
Copy link

codecov bot commented Jul 29, 2020

Codecov Report

Merging #5614 into master will increase coverage by 0.31%.
The diff coverage is 98.61%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #5614      +/-   ##
==========================================
+ Coverage   78.49%   78.81%   +0.31%     
==========================================
  Files         146      146              
  Lines       26335    26396      +61     
==========================================
+ Hits        20671    20803     +132     
+ Misses       5664     5593      -71     
Impacted Files Coverage Δ
src/transformers/__init__.py 99.24% <ø> (ø)
src/transformers/modeling_auto.py 78.48% <ø> (ø)
src/transformers/modeling_tf_flaubert.py 88.19% <94.73%> (+62.37%) ⬆️
src/transformers/modeling_flaubert.py 86.61% <100.00%> (+1.43%) ⬆️
src/transformers/modeling_tf_xlm.py 92.96% <100.00%> (+11.98%) ⬆️
src/transformers/modeling_xlm.py 91.00% <100.00%> (+0.98%) ⬆️
src/transformers/tokenization_xlm.py 16.26% <0.00%> (-66.67%) ⬇️
src/transformers/modeling_tf_gpt2.py 65.42% <0.00%> (-29.91%) ⬇️
src/transformers/tokenization_marian.py 68.14% <0.00%> (-25.67%) ⬇️
src/transformers/generation_tf_utils.py 79.94% <0.00%> (-3.76%) ⬇️
... and 7 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8a8ae27...dd85766. Read the comment docs.

@LysandreJik LysandreJik merged commit 3f94170 into master Jul 29, 2020
@LysandreJik LysandreJik deleted the test-tf-flaubert branch July 29, 2020 18:26
stas00 added a commit to stas00/transformers that referenced this pull request Jul 30, 2020
commit 54f9fbe
Author: Julien Plu <plu.julien@gmail.com>
Date:   Wed Jul 29 20:32:01 2020 +0200

    Rework TF trainer (huggingface#6038)

    * Fully rework training/prediction loops

    * fix method name

    * Fix variable name

    * Fix property name

    * Fix scope

    * Fix method name

    * Fix tuple index

    * Fix tuple index

    * Fix indentation

    * Fix variable name

    * fix eval before log

    * Add drop remainder for test dataset

    * Fix step number + fix logging datetime

    * fix eval loss value

    * use global step instead of step + fix logging at step 0

    * Fix logging datetime

    * Fix global_step usage

    * Fix breaking loop + logging datetime

    * Fix step in prediction loop

    * Fix step breaking

    * Fix train/test loops

    * Force TF at least 2.2 for the trainer

    * Use assert_cardinality to facilitate the dataset size computation

    * Log steps per epoch

    * Make tfds compliant with TPU

    * Make tfds compliant with TPU

    * Use TF dataset enumerate instead of the Python one

    * revert previous commit

    * Fix data_dir

    * Apply style

    * rebase on master

    * Address Sylvain's comments

    * Address Sylvain's and Lysandre comments

    * Trigger CI

    * Remove unused import

commit 3f94170
Author: Lysandre Debut <lysandre@huggingface.co>
Date:   Wed Jul 29 14:26:26 2020 -0400

    [WIP] Test TF Flaubert + Add {XLM, Flaubert}{TokenClassification, MultipleC… (huggingface#5614)

    * Test TF Flaubert + Add {XLM, Flaubert}{TokenClassification, MultipleChoice} models and tests

    * AutoModels

    Tiny tweaks

    * Style

    * Final changes before merge

    * Re-order for simpler review

    * Final fixes

    * Addressing @sgugger's comments

    * Test MultipleChoice

commit 8a8ae27
Author: Sylvain Gugger <35901082+sgugger@users.noreply.github.com>
Date:   Wed Jul 29 12:28:12 2020 -0400

    Use google style to document properties (huggingface#6130)

    * Use google style to document properties

    * Update src/transformers/configuration_utils.py

    Co-authored-by: Lysandre Debut <lysandre@huggingface.co>

    Co-authored-by: Lysandre Debut <lysandre@huggingface.co>

commit fc64559
Author: Julien Plu <plu.julien@gmail.com>
Date:   Wed Jul 29 18:20:00 2020 +0200

    Fix TF CTRL model naming (huggingface#6134)

commit 641b873
Author: Lysandre Debut <lysandre@huggingface.co>
Date:   Wed Jul 29 11:38:15 2020 -0400

    XLNet PLM Readme (huggingface#6121)

commit 8d157c9
Author: Timo Moeller <timo.moeller@deepset.ai>
Date:   Wed Jul 29 17:34:16 2020 +0200

    add deepset/xlm-roberta-large-squad2 model card (huggingface#6128)

    * Add xlm-r QA model card

    * Add tags

commit 6c00285
Author: Funtowicz Morgan <mfuntowicz@users.noreply.github.com>
Date:   Wed Jul 29 13:21:29 2020 +0200

    Added capability to quantize a model while exporting through ONNX. (huggingface#6089)

    * Added capability to quantize a model while exporting through ONNX.

    Signed-off-by: Morgan Funtowicz <morgan@huggingface.co>

    We do not support multiple extensions

    Signed-off-by: Morgan Funtowicz <morgan@huggingface.co>

    * Reformat files

    Signed-off-by: Morgan Funtowicz <morgan@huggingface.co>

    * More quality

    Signed-off-by: Morgan Funtowicz <morgan@huggingface.co>

    * Ensure test_generate_identified_name compares the same object types

    Signed-off-by: Morgan Funtowicz <morgan@huggingface.co>

    * Added documentation everywhere on ONNX exporter

    Signed-off-by: Morgan Funtowicz <morgan@huggingface.co>

    * Use pathlib.Path instead of plain-old string

    Signed-off-by: Morgan Funtowicz <morgan@huggingface.co>

    * Use f-string everywhere

    Signed-off-by: Morgan Funtowicz <morgan@huggingface.co>

    * Use the correct parameters for black formatting

    Signed-off-by: Morgan Funtowicz <morgan@huggingface.co>

    * Use Python 3 super() style.

    Signed-off-by: Morgan Funtowicz <morgan@huggingface.co>

    * Use packaging.version to ensure installed onnxruntime version match requirements

    Signed-off-by: Morgan Funtowicz <morgan@huggingface.co>

    * Fixing imports sorting order.

    Signed-off-by: Morgan Funtowicz <morgan@huggingface.co>

    * Missing raise(s)

    Signed-off-by: Morgan Funtowicz <morgan@huggingface.co>

    * Added quantization documentation

    Signed-off-by: Morgan Funtowicz <morgan@huggingface.co>

    * Fix some spelling.

    Signed-off-by: Morgan Funtowicz <morgan@huggingface.co>

    * Fix bad list header format

    Signed-off-by: Morgan Funtowicz <morgan@huggingface.co>

commit 25de74c
Author: Sylvain Gugger <35901082+sgugger@users.noreply.github.com>
Date:   Wed Jul 29 05:20:53 2020 -0400

    Use FutureWarning to deprecate (huggingface#6111)

commit 640550f
Author: Funtowicz Morgan <mfuntowicz@users.noreply.github.com>
Date:   Wed Jul 29 11:02:35 2020 +0200

    ONNX documentation (huggingface#5992)

    * Move torchscript and add ONNX documentation under modle_export

    Signed-off-by: Morgan Funtowicz <funtowiczmo@gmail.com>

    * Let's follow guidelines by the gurus: Renamed torchscript.rst to serialization.rst

    Signed-off-by: Morgan Funtowicz <funtowiczmo@gmail.com>

    * Remove previously introduced tree element

    Signed-off-by: Morgan Funtowicz <funtowiczmo@gmail.com>

    * WIP doc

    Signed-off-by: Morgan Funtowicz <funtowiczmo@gmail.com>

    * ONNX documentation

    Signed-off-by: Morgan Funtowicz <morgan@huggingface.co>

    * Fix invalid link

    Signed-off-by: Morgan Funtowicz <morgan@huggingface.co>

    * Improve spelling

    Signed-off-by: Morgan Funtowicz <morgan@huggingface.co>

    * Final wording pass

    Signed-off-by: Morgan Funtowicz <morgan@huggingface.co>

commit 92f8ce2
Author: Sam Shleifer <sshleifer@gmail.com>
Date:   Tue Jul 28 18:30:16 2020 -0400

    Fix deebert tests (huggingface#6102)

commit c49cd92
Author: Sam Shleifer <sshleifer@gmail.com>
Date:   Tue Jul 28 18:29:35 2020 -0400

    [Fix] position_ids tests again (huggingface#6100)

commit 40796c5
Author: Sam Shleifer <sshleifer@gmail.com>
Date:   Tue Jul 28 18:29:18 2020 -0400

    [fix] add bart to LM_MAPPING (huggingface#6099)

commit 5abe503
Author: Sam Shleifer <sshleifer@gmail.com>
Date:   Tue Jul 28 18:27:58 2020 -0400

    Fix huggingface#6096: MBartTokenizer's mask token (huggingface#6098)

commit b1c8b76
Author: Joe Davison <josephddavison@gmail.com>
Date:   Tue Jul 28 14:46:03 2020 -0400

    Fix zero-shot pipeline single seq output shape (huggingface#6104)

commit 06834bc
Author: Lysandre Debut <lysandre@huggingface.co>
Date:   Tue Jul 28 12:44:25 2020 -0400

    Logs should not be hidden behind a logger.info (huggingface#6097)

commit dafa296
Author: Sam Shleifer <sshleifer@gmail.com>
Date:   Tue Jul 28 11:24:23 2020 -0400

    [s2s] Delete useless method, log tokens_per_batch (huggingface#6081)

commit dc4755c
Author: Tanmay Thakur <thakurtanmay72@yahoo.com>
Date:   Tue Jul 28 19:30:23 2020 +0530

    create model-card for lordtt13/emo-mobilebert (huggingface#6030)

commit 28931f8
Author: Sylvain Gugger <35901082+sgugger@users.noreply.github.com>
Date:   Tue Jul 28 09:48:39 2020 -0400

    Fix huggingface#6092 (huggingface#6093)

    * Fix huggingface#6092

    * Format

commit 5e97c82
Author: Manuel Romero <mrm8488@gmail.com>
Date:   Tue Jul 28 15:36:00 2020 +0200

    Create README.md (huggingface#6076)

commit 54f49af
Author: Clement <clementdelangue@gmail.Com>
Date:   Tue Jul 28 09:14:00 2020 -0400

    Add inference widget examples (huggingface#5825)

commit 0206efb
Author: Sylvain Gugger <35901082+sgugger@users.noreply.github.com>
Date:   Tue Jul 28 09:08:20 2020 -0400

    Make all data collators accept dict (huggingface#6065)

    * Make all data collators accept dict

    * Style

commit 31a5486
Author: Sam Shleifer <sshleifer@gmail.com>
Date:   Tue Jul 28 08:41:27 2020 -0400

    github issue template suggests who to tag (huggingface#5790)

    Co-authored-by: Julien Chaumond <chaumond@gmail.com>
    Co-authored-by: Sylvain Gugger <35901082+sgugger@users.noreply.github.com>
    Co-authored-by: Lysandre Debut <lysandre@huggingface.co>
    Co-authored-by: Teven <teven.lescao@gmail.com>

commit f0c7008
Author: Stas Bekman <stas00@users.noreply.github.com>
Date:   Tue Jul 28 05:34:58 2020 -0700

    link to README.md (huggingface#6068)

    * add a link to README.md

    * Update README.md

commit 4f814fd
Author: Pavel Soriano <pavel.soriano@data.gouv.fr>
Date:   Tue Jul 28 14:33:52 2020 +0200

    [Model Card] camembert-base-squadFR-fquad-piaf (huggingface#6087)

commit 3c7fbf3
Author: Sam Shleifer <sshleifer@gmail.com>
Date:   Tue Jul 28 08:18:11 2020 -0400

    MBART: support summarization tasks where max_src_len > max_tgt_len (huggingface#6003)

    * MBART: support summarization tasks

    * fix test

    * Style

    * add tokenizer test

commit 842eb45
Author: Tanmay Thakur <thakurtanmay72@yahoo.com>
Date:   Tue Jul 28 13:55:12 2020 +0530

    New Community NB Add (huggingface#5824)

    Signed-off-by: lordtt13 <thakurtanmay72@yahoo.com>

commit 018d61f
Author: Andrés Felipe Cruz <bones.felipe@gmail.com>
Date:   Tue Jul 28 01:19:17 2020 -0700

    Moving transformers package import statements to relative imports in some files (huggingface#5796)

    * Moving rom transformers statements to relative imports in some files under src/

    * Import order

    Co-authored-by: Lysandre Debut <lysandre@huggingface.co>

commit 7214954
Author: Lysandre Debut <lysandre@huggingface.co>
Date:   Tue Jul 28 03:14:31 2020 -0400

    Should return a tuple for serialization (huggingface#6061)

commit 7a68d40
Author: Sam Shleifer <sshleifer@gmail.com>
Date:   Mon Jul 27 20:07:21 2020 -0400

    [s2s] Don't mention packed data in README (huggingface#6079)

commit b7345d2
Author: Sam Shleifer <sshleifer@gmail.com>
Date:   Mon Jul 27 20:00:44 2020 -0400

    [fix] no warning for position_ids buffer (huggingface#6063)

commit 1e00ef6
Author: Sam Shleifer <sshleifer@gmail.com>
Date:   Mon Jul 27 18:26:00 2020 -0400

    [s2s] dont document packing because it hurts performance (huggingface#6077)

commit 9d0d3a6
Author: sgugger <sylvain.gugger@gmail.com>
Date:   Mon Jul 27 18:03:09 2020 -0400

    Pin TF while we wait for a fix

commit 769e6ba
Author: Ramsri Goutham Golla <ramsrigouthamg@gmail.com>
Date:   Tue Jul 28 01:55:37 2020 +0530

    Create README.md (huggingface#6032)

    Adding model card - readme
sgugger added a commit that referenced this pull request Jul 30, 2020
* Switch from return_tuple to return_dict

* Fix test

* [WIP] Test TF Flaubert + Add {XLM, Flaubert}{TokenClassification, MultipleC… (#5614)

* Test TF Flaubert + Add {XLM, Flaubert}{TokenClassification, MultipleChoice} models and tests

* AutoModels


Tiny tweaks

* Style

* Final changes before merge

* Re-order for simpler review

* Final fixes

* Addressing @sgugger's comments

* Test MultipleChoice

* Rework TF trainer (#6038)

* Fully rework training/prediction loops

* fix method name

* Fix variable name

* Fix property name

* Fix scope

* Fix method name

* Fix tuple index

* Fix tuple index

* Fix indentation

* Fix variable name

* fix eval before log

* Add drop remainder for test dataset

* Fix step number + fix logging datetime

* fix eval loss value

* use global step instead of step + fix logging at step 0

* Fix logging datetime

* Fix global_step usage

* Fix breaking loop + logging datetime

* Fix step in prediction loop

* Fix step breaking

* Fix train/test loops

* Force TF at least 2.2 for the trainer

* Use assert_cardinality to facilitate the dataset size computation

* Log steps per epoch

* Make tfds compliant with TPU

* Make tfds compliant with TPU

* Use TF dataset enumerate instead of the Python one

* revert previous commit

* Fix data_dir

* Apply style

* rebase on master

* Address Sylvain's comments

* Address Sylvain's and Lysandre comments

* Trigger CI

* Remove unused import

* Switch from return_tuple to return_dict

* Fix test

* Add recent model

Co-authored-by: Lysandre Debut <lysandre@huggingface.co>
Co-authored-by: Julien Plu <plu.julien@gmail.com>
@sgugger sgugger mentioned this pull request Aug 24, 2020
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.

None yet

3 participants