Skip to content

Conversation

@sshleifer
Copy link
Contributor

@sshleifer sshleifer commented Apr 6, 2020

Speedup torch summarization tests by using small models that are faster to download and instantiate.

@sshleifer
Copy link
Contributor Author

Non slow test speed before change:
image

@patrickvonplaten
Copy link
Contributor

There is a tiny TF T5 model now as well via:
model = TFAutoModelWithLMHead.from_pretrained("patrickvonplaten/t5-tiny-random")

@codecov-io
Copy link

codecov-io commented Apr 7, 2020

Codecov Report

Merging #3663 into master will decrease coverage by 0.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3663      +/-   ##
==========================================
- Coverage   78.03%   78.02%   -0.02%     
==========================================
  Files         104      104              
  Lines       17708    17708              
==========================================
- Hits        13819    13817       -2     
- Misses       3889     3891       +2     
Impacted Files Coverage Δ
src/transformers/modeling_tf_utils.py 92.96% <0.00%> (-0.17%) ⬇️
src/transformers/modeling_utils.py 91.97% <0.00%> (-0.13%) ⬇️

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 0a9d09b...4751188. Read the comment docs.

@julien-c
Copy link
Member

julien-c commented Apr 7, 2020

LGTM, but FYI for context, I think the reason we were using the real models was that we intended to do integration testing: in _test_mono_column_pipeline we only test equality of keys but we could have tested equality (or closeness) of values.

@sshleifer sshleifer merged commit 0a4b106 into huggingface:master Apr 7, 2020
@sshleifer sshleifer deleted the faster-pipelines-test branch April 7, 2020 18:01
@sshleifer
Copy link
Contributor Author

Makes sense @julien-c . I'd be happy to add some @slow integration tests and try to fulfill the original intent

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.

4 participants