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

Fixing model checkpoints to be robust to mutli -> single GPU usage #1091

Open
jeswan opened this issue Sep 17, 2020 · 3 comments
Open

Fixing model checkpoints to be robust to mutli -> single GPU usage #1091

jeswan opened this issue Sep 17, 2020 · 3 comments

Comments

@jeswan
Copy link
Contributor

jeswan commented Sep 17, 2020

Issue by pruksmhc
Sunday May 17, 2020 at 17:32 GMT
Originally opened as nyu-mll/jiant#1091


This is a fix to nyu-mll/jiant#1087. I decided to make the change in the model loading portion because making the change in model saving as suggested in the nyu-mll/jiant#1087 will fix multi -> single GPU model loading, but will break multi -> multi GPU model loading (if we want to reload a checkpoint that was trained in multi-GPU on a multi-GPU machine).
Additionally, I also did some light cleanup of model loading in trainer to not be redundant, and also deleted an unused parameter.

Tests
Multi -> Single GPU: I tested by training a roberta-large model on SST on multi-GPU, and then loading that checkpoint in a single-GPU for further training.
Multi -> Multi GPU: This is implicitly already done in jiant, specifically we load the best checkpoint before doing evaluation, so this was tested when I trained the roberta-large SST model the first time on multi-GPU.


pruksmhc included the following code: https://github.com/nyu-mll/jiant/pull/1091/commits

@jeswan
Copy link
Contributor Author

jeswan commented Sep 17, 2020

Comment by pep8speaks
Sunday May 17, 2020 at 17:32 GMT


Hello @pruksmhc! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

Line 541:55: W291 trailing whitespace
Line 548:12: W291 trailing whitespace

Line 329:77: W291 trailing whitespace

You can repair most issues by installing black and running: black -l 100 ./*. If you contribute often, have a look at the 'Contributing' section of the README for instructions on doing this automatically.

Comment last updated at 2020-05-18 20:46:53 UTC

@jeswan
Copy link
Contributor Author

jeswan commented Sep 17, 2020

Comment by pyeres
Tuesday May 19, 2020 at 19:00 GMT


Hi @zphang & @HaokunLiu — are either of you available to provide the substantial review for this PR? The core concerns seem to be 1) whether this addresses issue #1087, and 2) whether these changes introduce new risks/regressions.

@jeswan
Copy link
Contributor Author

jeswan commented Sep 17, 2020

Comment by jeswan
Thursday Sep 17, 2020 at 16:09 GMT


Are these changes still necessary? Planning to close all PRs to move jiant2 to this repo in the near future.

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

No branches or pull requests

1 participant