HuggingFace improvements #649

daavoo · 2023-08-04T13:46:31Z

Log Trainer.args.
Add log_model argument.
Every framework does its own thing. No strong opinions but I went with the following:
- If None (default) will not log any artifact.
- If all will call log_artifact with output_dir at each on_save call.
- If True will save the model on_train_end and call log_artifact with type=model and copy=True.
  Will use best as name if args.load_best_model_at_end else last.
Add Notebook/Colab example

Closes #641

TODO:

dvc.org updates

- If `None` (default) will not log any artifact. - If `all` will call log_artifact with `output_dir` at each `on_save` call. - If `last` will save the model `on_train_end` and call `log_artifact` with type=model and copy=True.

src/dvclive/huggingface.py

dberenbaum · 2023-08-05T13:01:03Z

src/dvclive/huggingface.py

-            self.live.log_artifact(args.output_dir)
+            output_dir = os.path.join(args.output_dir, "last")
+            fake_trainer.save_model(output_dir)
+            self.live.log_artifact(output_dir, type="model", copy=True)


Cross-framework consistency isn't our highest priority, but should we agree on some common principles for the final artifact, like naming and whether to copy it?

I would like for all the integrations to have just 2 options:

all/checkpoints: resuming scenarios.
Log the entire checkpoint folder

best: model registry
Log the best checkpoint on end with copy=True, name="best", type="model"

That's fine with me. Do you want to update the lightning logger to use copy=True? AFAIK the rest is consistent.

will update this and lightning to use that

Sorry, are you also suggesting to change the behavior of log_model=True in lightning to track only the copied best artifact and not the whole directory? That's fine, just want to make sure I understand what you mean.

For HF, how should we handle the last/best checkpoint? If args.load_best_model_at_end, we could add name=best? WDYT?

Sorry, are you also suggesting to change the behavior of log_model=True in lightning to track only the copied best artifact and not the whole directory

I think I would suggest dropping the boolean value.

For HF, how should we handle the last/best checkpoint? If args.load_best_model_at_end, we could add name=best? WDYT?

Yes, makes sense.

I think I would suggest dropping the boolean value.

I worry doing that and/or not saving the checkpoints dir breaks consistency with mlflow/wandb/etc. in lightning for the sake of consistency across dvclive. I would probably err on the side of sticking with consistency for lightning over consistency for dvclive where they conflict, but we can always make this a follow-up PR if it is taking this off track.

or HF, how should we handle the last/best checkpoint? If args.load_best_model_at_end, we could add name=best? WDYT?

Updated with this behavior

Also, dropped last option in favor of True

src/dvclive/huggingface.py

dberenbaum

Have a few questions where we need to align.

As far as breaking changes, maybe we should go ahead and make the easy ones from the 3.0 checklist and do a release. I think we will need a major version where we transition to log_model and moving callbacks into frameworks and can't do it all at once anyway. The alternative is likely to branch-based development and only make the breaking changes available on the 3.0 branch. WDYT?

codecov-commenter · 2023-08-07T08:31:58Z

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.17% 🎉

Comparison is base (786c83a) 88.06% compared to head (fbf8865) 88.24%.
Report is 11 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #649      +/-   ##
==========================================
+ Coverage   88.06%   88.24%   +0.17%     
==========================================
  Files          43       43              
  Lines        3042     3088      +46     
  Branches      260      270      +10     
==========================================
+ Hits         2679     2725      +46     
+ Misses        324      323       -1     
- Partials       39       40       +1

Files Changed	Coverage Δ
src/dvclive/huggingface.py	`100.00% <100.00%> (+6.89%)`	⬆️
tests/test_frameworks/test_huggingface.py	`95.95% <100.00%> (-0.27%)`	⬇️

... and 4 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

SoyGema

Thanks for making an example! 🙏Would like to propose some additions if you don´t mind. I can´t comment directly due to the notebook nature, so hopefully this can help. If you are time sensitive, and agree on the proposed additions/changes, happy to submit a contrib.🎺

Add # Goal/Intro section
Cover the full functionality the example is exploring at the beginning of the notebook

Proposal : Example of fine-tuning sentiment analysis classifier based on imbd and distilbert pretrained model , experiment tracking and results metrics analysis with dvclive and dvc api

Add section # Initialize git and dvc in 2nd cell
why? It gives context about the necessary requirements beyond the libraries install that a data scientist needs to run some other HF examples
Change # Dataset for # Dataset and Tokenization in 3rd cell
The section includes a Tokenization process
Add # Evaluation metrics section
For consistency / coherence with respect to the process
Possible discussion L28
Describe in comment what log_model does for user? which seems to be the improvement of the PR?
Explain # Comparing section
Context : unclear right now whats really going on with naming . I´ve lost a few chapters of this exploring other things . Thinking about sharing in #9709 . When exploring this, a question came into my mind, what is the difference in between dvc.api and dvclive ?

daavoo · 2023-08-15T09:23:37Z

Thanks for making an example! 🙏Would like to propose some additions if you don´t mind. I can´t comment directly due to the notebook nature, so hopefully this can help. If you are time sensitive, and agree on the proposed additions/changes, happy to submit a contrib.🎺

Thanks @SoyGema ! All points make sense to me. I have opened a separate issue to address as a follow-up. I believe it applies to all the examples and not only to the huggingface one.

daavoo · 2023-08-15T09:33:50Z

As far as breaking changes, maybe we should go ahead and make the easy ones from the 3.0 checklist and do a release. I think we will need a major version where we transition to log_model and moving callbacks into frameworks and can't do it all at once anyway. The alternative is likely to branch-based development and only make the breaking changes available on the 3.0 branch. WDYT?

I kept the model_file behavior with a warning about deprecation for now

per iterative/dvclive#649

* dvclive: Add huggingface updates per iterative/dvclive#649 * updates from review

daavoo added 3 commits August 2, 2023 12:27

huggingface: log some parameters

912a077

huggingface: Add log_model.

24fb584

- If `None` (default) will not log any artifact. - If `all` will call log_artifact with `output_dir` at each `on_save` call. - If `last` will save the model `on_train_end` and call `log_artifact` with type=model and copy=True.

examples: Add DVCLive-HuggingFace notebook

8cac907

daavoo added feature A: frameworks Area: ML Framework integration labels Aug 4, 2023

daavoo self-assigned this Aug 4, 2023

daavoo requested a review from dberenbaum August 4, 2023 13:46

dberenbaum reviewed Aug 5, 2023

View reviewed changes

src/dvclive/huggingface.py Outdated Show resolved Hide resolved

dberenbaum reviewed Aug 5, 2023

View reviewed changes

src/dvclive/huggingface.py Outdated Show resolved Hide resolved

dberenbaum reviewed Aug 5, 2023

View reviewed changes

Don't cherry-pick args

fbf8865

SoyGema reviewed Aug 8, 2023

View reviewed changes

dberenbaum mentioned this pull request Aug 14, 2023

lightning: copy best model #659

Merged

huggingface: Conditional model name based on load_best_model_at_end

172290c

daavoo mentioned this pull request Aug 15, 2023

examples: Add details and improve descriptions #663

Closed

huggingface: Keep model_file behavior

5a0e750

daavoo requested a review from dberenbaum August 15, 2023 09:35

daavoo force-pushed the huggingface-log-model branch from a88e9e7 to 07dbe4e Compare August 15, 2023 10:03

Use True instead of last.

2c03182

daavoo force-pushed the huggingface-log-model branch from 07dbe4e to 2c03182 Compare August 15, 2023 10:13

dberenbaum approved these changes Aug 15, 2023

View reviewed changes

daavoo merged commit f1b8e2a into main Aug 16, 2023
11 checks passed

daavoo deleted the huggingface-log-model branch August 16, 2023 09:31

dberenbaum mentioned this pull request Aug 16, 2023

dvclive: add huggingface log_model arg iterative/dvc.org#4770

Closed

daavoo added a commit to iterative/dvc.org that referenced this pull request Aug 18, 2023

dvclive: Add huggingface updates

85165e0

per iterative/dvclive#649

daavoo mentioned this pull request Aug 18, 2023

dvclive: Add huggingface updates iterative/dvc.org#4779

Merged

daavoo added a commit to iterative/dvc.org that referenced this pull request Aug 18, 2023

dvclive: Add huggingface updates

a90cf9c

per iterative/dvclive#649

daavoo added a commit to iterative/dvc.org that referenced this pull request Aug 18, 2023

dvclive: Add huggingface updates (#4779)

9abf107

* dvclive: Add huggingface updates per iterative/dvclive#649 * updates from review

SoyGema mentioned this pull request Aug 24, 2023

Story : Plots , Metrics for LLM support [2] iterative/dvc-render#138

Closed

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

HuggingFace improvements #649

HuggingFace improvements #649

daavoo commented Aug 4, 2023 •

edited

Loading

dberenbaum Aug 5, 2023

daavoo Aug 7, 2023 •

edited

Loading

dberenbaum Aug 7, 2023

daavoo Aug 7, 2023

dberenbaum Aug 7, 2023

daavoo Aug 7, 2023

dberenbaum Aug 7, 2023

daavoo Aug 15, 2023

daavoo Aug 15, 2023

dberenbaum left a comment

codecov-commenter commented Aug 7, 2023

SoyGema left a comment

daavoo commented Aug 15, 2023

daavoo commented Aug 15, 2023

HuggingFace improvements #649

HuggingFace improvements #649

Conversation

daavoo commented Aug 4, 2023 • edited Loading

Choose a reason for hiding this comment

daavoo Aug 7, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dberenbaum left a comment

Choose a reason for hiding this comment

codecov-commenter commented Aug 7, 2023

Codecov Report

SoyGema left a comment

Choose a reason for hiding this comment

daavoo commented Aug 15, 2023

daavoo commented Aug 15, 2023

daavoo commented Aug 4, 2023 •

edited

Loading

daavoo Aug 7, 2023 •

edited

Loading