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

Update to Transformers v4.3.3 #1266

Merged
merged 5 commits into from
Feb 25, 2021

Conversation

jeswan
Copy link
Collaborator

@jeswan jeswan commented Jan 14, 2021

Updating to Transformers v4.3.3 enables fast (Rust) tokenizers by default. Breaking changes found here are addressed in this PR including:

  • Transformer model folder update
  • Switching to default return_dict (as opposed to returned tuples)
  • Renaming tokenizer member variables

@jeswan jeswan marked this pull request as draft January 14, 2021 18:56
@jeswan jeswan force-pushed the js/maint/update_transformers_v4.2.1 branch from e24c84a to 20913e4 Compare January 14, 2021 18:58
@jeswan jeswan marked this pull request as ready for review January 14, 2021 18:58
@codecov
Copy link

codecov bot commented Jan 14, 2021

Codecov Report

Merging #1266 (6404f13) into js/feature/easy_add_model (6e6c2e3) will increase coverage by 0.05%.
The diff coverage is 34.61%.

Impacted file tree graph

@@                      Coverage Diff                      @@
##           js/feature/easy_add_model    #1266      +/-   ##
=============================================================
+ Coverage                      55.51%   55.56%   +0.05%     
=============================================================
  Files                            149      148       -1     
  Lines                          10903    10884      -19     
=============================================================
- Hits                            6053     6048       -5     
+ Misses                          4850     4836      -14     
Impacted Files Coverage Δ
jiant/proj/main/modeling/heads.py 38.93% <0.00%> (ø)
jiant/tasks/lib/ropes.py 20.39% <0.00%> (ø)
jiant/proj/main/modeling/taskmodels.py 38.31% <41.17%> (-0.76%) ⬇️
jiant/tasks/lib/templates/squad_style/core.py 30.71% <50.00%> (ø)
jiant/tasks/lib/templates/squad_style/utils.py 9.37% <100.00%> (ø)

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 6e6c2e3...6404f13. Read the comment docs.

@zphang
Copy link
Collaborator

zphang commented Jan 14, 2021

  • We should switch to using the default return_dict in the taskmodels
  • Worth updating transformer_utils.

attention_mask=input_mask,
output_hidden_states=output_hidden_states,
)
return output.pooler_output, output.last_hidden_state, output
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@zphang Do you know what the expected contents of other is? I'm currently looking at https://huggingface.co/transformers/_modules/transformers/modeling_bert.html. I think I need to make an additional change so that other contains the expected values (used here: https://github.com/nyu-mll/jiant/blob/master/jiant/proj/main/modeling/taskmodels.py#L240)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Looking at this again...it looks like other should be the hidden_states. Let me know if I am missing anything.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Hidden states/attention weights. One major benefit of using return_dict is to be able to do away with "other".

@jeswan jeswan force-pushed the js/maint/update_transformers_v4.2.1 branch from be42ff0 to c897a62 Compare January 19, 2021 05:48
@jeswan jeswan force-pushed the js/maint/update_transformers_v4.2.1 branch from c897a62 to d6d989c Compare January 19, 2021 05:59
@jeswan jeswan changed the base branch from master to js/feature/easy_add_model February 2, 2021 07:56
@vblagoje
Copy link

Hey guys, do you have a merge ETA for this PR? TIA

@jeswan
Copy link
Collaborator Author

jeswan commented Feb 16, 2021

@vblagoje Planning to get this PR into master within a week

@jeswan jeswan changed the title Update to Transformers v4.2.1 Update to Transformers v4.3.3 Feb 25, 2021
@jeswan jeswan merged commit b2cfb2a into js/feature/easy_add_model Feb 25, 2021
@jeswan jeswan deleted the js/maint/update_transformers_v4.2.1 branch February 25, 2021 15:14
@jeswan
Copy link
Collaborator Author

jeswan commented Feb 25, 2021

@vblagoje please checkout the feature branch js/feature/easy_add_model for now while we test these changes

jeswan added a commit that referenced this pull request May 4, 2021
* Update to Transformers v4.3.3 (#1266)

* use default return_dict in taskmodels and remove hidden state context manager in models.

* return hidden states in output of model wrapper

* Switch to task model/head factories instead of embedded if-else statements (#1268)

* Use jiant transformers model wrapper instead of if-else. Use taskmodel and head factory instead of if-else.

* switch to ModelArchitectures enum instead of strings

* Refactor get_output_from_encoder() to be member of JiantTaskModel (#1283)

* refactor getting output from encoder to be member function of jiant model

* switch to explicit encode() in jiant transformers model

* fix simple runscript test

* update to tokenizer 0.10.1

* Add tests for flat_strip() (#1289)

* add flat_strip test

* add list to test cases flat_strip

* mlm_weights(), feat_spec(), flat_strip() if-else refactors (#1288)

* moves remaining if-else statments to jiant model or replaces with model agnostic method

* switch from jiant_transformers_model to encoder

* fix bug in flat_strip()

* Move tokenization logic to central JiantModelTransformers method (#1290)

* move model specific tokenization logic to JiantTransformerModels

* implement abstract methods for JiantTransformerModels

* fix tasks circular import (#1296)

* Add DeBERTa (#1295)

* Add DeBERTa with sanity test

* fix tasks circular import

* [WIP] add deberta tests

* Revert "fix tasks circular import"

This reverts commit f924640.

* deberta tests passing with transformers 6472d8

* switch to deberta-v2

* fix get_mlm_weights_dict() for deberta-v2

* update to transformers 4.5.0

* mark deberta test_export as slow

* Update test_tokenization_normalization.py

* add guide to add a model

* fix test_expor_model tests

* minor pytest fixes (add num_labels for rte, overnight flag fix)

* bugfix for simple api notebook

* bugfix for #1310

* bugfix for #1306: simple api notebook path name

* squad running

* 2nd bugfix for #1310: not all tasks have num_labels property

* simple api notebook back to roberta-base

* run test matrix for more steps to compare to master

* save last/best model test fix

Co-authored-by: Jesse Swanson <js11133Wnyu.edu>
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.

Update to Transformers v4
3 participants