transformers icon indicating copy to clipboard operation
transformers copied to clipboard

[neptune] fix checkpoint bug with relative out_dir

Open kshitij12345 opened this issue 2 years ago • 4 comments

What does this PR do?

It takes care of the following:

  • Updates to NeptuneCallback aligned with Neptune 1.0 release : https://docs.neptune.ai/setup/neptune-client_1-0_release_changes/

  • It also fixes the case where we silently don't log the model_checkpoints when output_dir argument has a relative path of the form ../models. Ref to the relevant lines of code: https://github.com/huggingface/transformers/blob/3be0e6e4a367dadb453ac31dad46fb665dc28b42/src/transformers/integrations.py#L1352-L1354 https://github.com/huggingface/transformers/blob/3be0e6e4a367dadb453ac31dad46fb665dc28b42/src/transformers/integrations.py#L1296-L1305

Before submitting

  • [ ] This PR fixes a typo or improves the docs (you can dismiss the other checks if that's the case).
  • [ ] Did you read the contributor guideline, Pull Request section?
  • [ ] Was this discussed/approved via a Github issue or the forum? Please add a link to it if that's the case.
  • [ ] Did you make sure to update the documentation with your changes? Here are the documentation guidelines, and here are tips on formatting docstrings.
  • [ ] Did you write any new necessary tests?

Who can review?

Anyone in the community is free to review the PR once the tests have passed. Feel free to tag members/contributors who may be interested in your PR.

kshitij12345 avatar Mar 11 '23 07:03 kshitij12345

The documentation is not available anymore as the PR was closed or merged.

@sgugger Thanks for taking a look. Will update the PR description in sometime (probably today or early tomorrow) to have more information. Will ping once that is done.

kshitij12345 avatar Mar 16 '23 13:03 kshitij12345

@sgugger have updated the PR description. Let me know if that gives enough context. Thank you :)

kshitij12345 avatar Mar 17 '23 05:03 kshitij12345

@sgugger Thanks for the review. @AleksanderWWW is out for this week so we will update the PR once he is back (as he has more context on update for latest neptune version support).

Thank you :)!

kshitij12345 avatar Mar 22 '23 12:03 kshitij12345

Thanks! I believe now all the failing checks will be solved once you rebase your PR on the main branch.

sgugger avatar Mar 27 '23 15:03 sgugger

Thanks! I believe now all the failing checks will be solved once you rebase your PR on the main branch.

@sgugger Thank you for your support

AleksanderWWW avatar Mar 27 '23 16:03 AleksanderWWW