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.

Sign up for GitHub

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

Jump to bottom

[Unified TorchTrainer] Add HF Transformers TorchTrainer Utilities #38083

Merged
merged 31 commits into from
Aug 12, 2023
Merged

[Unified TorchTrainer] Add HF Transformers TorchTrainer Utilities #38083

merged 31 commits into from
Aug 12, 2023

Conversation

woshiyyya
Copy link
Member

@woshiyyya woshiyyya commented Aug 3, 2023 •
edited
Loading

Why are these changes needed?

Add HF Transformers related utilities for Unified TorchTrainer.

Rendered doc: https://anyscale-ray--38083.com.readthedocs.build/en/38083/

  • Add RayTrainReportCallback and prepare_trainer API for transformer integration
    • Add Unit tests for these APIs
  • Add one basic example using transformers.trainer + Ray TorchTrainer
  • Update user guides for Transformers + TorchTrainer
    • Getting start page
    • API page
    • Checkpoint loading and saving user guides
    • Data Loading and Ingestion user guides

Related issue number

Checks

  • I've signed off every commit(by using the -s flag, i.e., git commit -s) in this PR.
  • I've run scripts/format.sh to lint the changes in this PR.
  • I've included any doc changes needed for https://docs.ray.io/en/master/.
    • I've added any new APIs to the API Reference. For example, if I added a
      method in Tune, I've added it in doc/source/tune/api/ under the
      corresponding .rst file.
  • I've made sure the tests are passing. Note that there might be a few flaky tests, see the recent failures at https://flakey-tests.ray.io/
  • Testing Strategy
    • Unit tests
    • Release tests
    • This PR is not tested :(

woshiyyya and others added 3 commits August 3, 2023 11:56
@woshiyyya
init
e8ea452 
Signed-off-by: woshiyyya <xiaoyunxuan1998@gmail.com>
@woshiyyya
Merge branch 'master' into train/unified-api/add_transformers_utilities
3e6eabd
@woshiyyya
update utilities
57e458b 
Signed-off-by: woshiyyya <xiaoyunxuan1998@gmail.com>
@woshiyyya woshiyyya marked this pull request as ready for review August 9, 2023 01:28
@woshiyyya
polish code
690bdf7 
Signed-off-by: woshiyyya <xiaoyunxuan1998@gmail.com>
@woshiyyya
polish code
6ed560b 
Signed-off-by: woshiyyya <xiaoyunxuan1998@gmail.com>
@woshiyyya
Merge remote-tracking branch 'upstream/master' into train/unified-api…
9730923 
…/add_transformers_utilities
@woshiyyya
finish
d562460 
Signed-off-by: woshiyyya <xiaoyunxuan1998@gmail.com>
@woshiyyya woshiyyya force-pushed the train/unified-api/add_transformers_utilities branch from 7957cf1 to d562460 Compare August 10, 2023 17:57
@woshiyyya
add UT
4d4ecf6 
Signed-off-by: woshiyyya <xiaoyunxuan1998@gmail.com>
@woshiyyya
fix lint
fa41ab1 
Signed-off-by: woshiyyya <xiaoyunxuan1998@gmail.com>
@woshiyyya
add unittests
8cc2c9c 
Signed-off-by: woshiyyya <xiaoyunxuan1998@gmail.com>
@woshiyyya
fix tests
07b9953 
Signed-off-by: woshiyyya <xiaoyunxuan1998@gmail.com>
@woshiyyya
add basic example
7f32f04 
Signed-off-by: woshiyyya <xiaoyunxuan1998@gmail.com>
woshiyyya and others added 7 commits August 10, 2023 19:10
@woshiyyya
Merge branch 'master' into train/unified-api/add_transformers_utilities
3655a58
@woshiyyya
fix doc
1fd6695 
Signed-off-by: woshiyyya <xiaoyunxuan1998@gmail.com>
@woshiyyya
fix import path
4c6ccb5 
Signed-off-by: woshiyyya <xiaoyunxuan1998@gmail.com>
@woshiyyya
Merge remote-tracking branch 'upstream/master' into train/unified-api…
29d44a8 
…/add_transformers_utilities
@woshiyyya
wip
fcac881 
Signed-off-by: woshiyyya <xiaoyunxuan1998@gmail.com>
@woshiyyya
update getting start page
93f12c5 
Signed-off-by: woshiyyya <xiaoyunxuan1998@gmail.com>
@woshiyyya
add to toc tree
19ee0a1 
Signed-off-by: woshiyyya <xiaoyunxuan1998@gmail.com>
@woshiyyya
add report and data integration
8fdf7c6 
Signed-off-by: woshiyyya <xiaoyunxuan1998@gmail.com>
@woshiyyya
fix doc
a5e5cfe 
Signed-off-by: woshiyyya <xiaoyunxuan1998@gmail.com>
@woshiyyya
Merge remote-tracking branch 'upstream/master' into train/unified-api…
8602ff2 
…/add_transformers_utilities
@woshiyyya
update report callback
8ad6e11 
Signed-off-by: woshiyyya <xiaoyunxuan1998@gmail.com>
matthewdeng
matthewdeng reviewed Aug 11, 2023
View reviewed changes
Copy link
Contributor

@matthewdeng matthewdeng left a comment

Choose a reason for hiding this comment

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

This is awesome

python/ray/train/huggingface/transformers/_transformers_utils.py Show resolved Hide resolved
doc/source/_toc.yml Outdated Show resolved Hide resolved
python/ray/train/huggingface/__init__.py Outdated Show resolved Hide resolved
doc/source/train/user-guides/checkpoints.rst Outdated Show resolved Hide resolved
doc/source/train/user-guides/checkpoints.rst Outdated
Comment on lines 220 to 222
You should properly configure the `logging_strategy`, `save_strategy`
and `evaluation_strategy`, so that at the checkpoint saving step, transformers
trainer also reports the latest monitoring metrics (e.g. `eval_loss` in the above case).
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it would be helpful to include these directly in the example itself, as it's currently not being shown.

In other words, the example should make it clear that our callback is complementary to Transformers logging/checkpointing.

Copy link
Member Author

Choose a reason for hiding this comment

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

Make sense. Like explicitly define the save_strategy and evaluation_strategy, and put these info as comments in the example?

doc/source/train/user-guides/checkpoints.rst Outdated Show resolved Hide resolved
doc/source/train/user-guides/checkpoints.rst Outdated Show resolved Hide resolved
doc/source/train/user-guides/data-loading-preprocessing.rst Outdated Show resolved Hide resolved
woshiyyya and others added 4 commits August 11, 2023 12:04
@woshiyyya @matthewdeng
Apply suggestions from code review
1722273 
Co-authored-by: matthewdeng <matthew.j.deng@gmail.com>
Signed-off-by: Yunxuan Xiao <xiaoyunxuan1998@gmail.com>
@woshiyyya
address comments
ba8bdd8 
Signed-off-by: woshiyyya <xiaoyunxuan1998@gmail.com>
@woshiyyya
polish the basic example
4f496a3 
Signed-off-by: woshiyyya <xiaoyunxuan1998@gmail.com>
@woshiyyya
address comments
f35a499 
Signed-off-by: woshiyyya <xiaoyunxuan1998@gmail.com>
@woshiyyya
minor fix
7427d98 
Signed-off-by: woshiyyya <xiaoyunxuan1998@gmail.com>
@woshiyyya
fix Title underline too short
dcaca15 
Signed-off-by: woshiyyya <xiaoyunxuan1998@gmail.com>
@woshiyyya
fix lazy import and update checkpoint user guides
18cbca8 
Signed-off-by: woshiyyya <xiaoyunxuan1998@gmail.com>
@woshiyyya
update emphasize lines
88c7acf 
Signed-off-by: woshiyyya <xiaoyunxuan1998@gmail.com>
@matthewdeng matthewdeng merged commit 3d69d15 into ray-project:master Aug 12, 2023
108 of 117 checks passed
NripeshN pushed a commit to NripeshN/ray that referenced this pull request Aug 15, 2023
@woshiyyya @matthewdeng @NripeshN
[Unified TorchTrainer] Add HF Transformers TorchTrainer Utilities (ra…
ff56056 
…y-project#38083)

Signed-off-by: woshiyyya <xiaoyunxuan1998@gmail.com>
Signed-off-by: Yunxuan Xiao <xiaoyunxuan1998@gmail.com>
Co-authored-by: matthewdeng <matthew.j.deng@gmail.com>
Signed-off-by: NripeshN <nn2012@hw.ac.uk>
harborn pushed a commit to harborn/ray that referenced this pull request Aug 17, 2023
@woshiyyya @matthewdeng @harborn
[Unified TorchTrainer] Add HF Transformers TorchTrainer Utilities (ra…
f17af23 
…y-project#38083)

Signed-off-by: woshiyyya <xiaoyunxuan1998@gmail.com>
Signed-off-by: Yunxuan Xiao <xiaoyunxuan1998@gmail.com>
Co-authored-by: matthewdeng <matthew.j.deng@gmail.com>
Signed-off-by: harborn <gangsheng.wu@intel.com>
harborn pushed a commit to harborn/ray that referenced this pull request Aug 17, 2023
@woshiyyya @matthewdeng @harborn
[Unified TorchTrainer] Add HF Transformers TorchTrainer Utilities (ra…
2c4bc38 
…y-project#38083)

Signed-off-by: woshiyyya <xiaoyunxuan1998@gmail.com>
Signed-off-by: Yunxuan Xiao <xiaoyunxuan1998@gmail.com>
Co-authored-by: matthewdeng <matthew.j.deng@gmail.com>
This was referenced Aug 21, 2023
Closed
Closed
arvind-chandra pushed a commit to lmco/ray that referenced this pull request Aug 31, 2023
@woshiyyya @matthewdeng @arvind-chandra
[Unified TorchTrainer] Add HF Transformers TorchTrainer Utilities (ra…
236e24d 
…y-project#38083)

Signed-off-by: woshiyyya <xiaoyunxuan1998@gmail.com>
Signed-off-by: Yunxuan Xiao <xiaoyunxuan1998@gmail.com>
Co-authored-by: matthewdeng <matthew.j.deng@gmail.com>
Signed-off-by: e428265 <arvind.chandramouli@lmco.com>
vymao pushed a commit to vymao/ray that referenced this pull request Oct 11, 2023
@woshiyyya @matthewdeng
[Unified TorchTrainer] Add HF Transformers TorchTrainer Utilities (ra…
50218eb 
…y-project#38083)

Signed-off-by: woshiyyya <xiaoyunxuan1998@gmail.com>
Signed-off-by: Yunxuan Xiao <xiaoyunxuan1998@gmail.com>
Co-authored-by: matthewdeng <matthew.j.deng@gmail.com>
Signed-off-by: Victor <vctr.y.m@example.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@matthewdeng matthewdeng matthewdeng approved these changes

@richardliaw richardliaw Awaiting requested review from richardliaw

@krfricke krfricke Awaiting requested review from krfricke

@xwjiang2010 xwjiang2010 Awaiting requested review from xwjiang2010

@amogkam amogkam Awaiting requested review from amogkam

@Yard1 Yard1 Awaiting requested review from Yard1

@maxpumperla maxpumperla Awaiting requested review from maxpumperla

Assignees

@matthewdeng matthewdeng

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@woshiyyya @matthewdeng