Adds dvclive tracker #2139
Adds dvclive tracker #2139
Conversation
|
The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. |
dberenbaum
force-pushed
the
dvclive-callback
branch
from
f5d67f1 to
a13d0be
Compare
Thanks for catching that! Added it in the latest commit. |
|
Will take a look at the test failures |
src/accelerate/tracking.py
Outdated
| A `Tracker` class that supports `dvclive`. Should be initialized at the start of your script. | ||
|
|
||
| Args: | ||
| run_name (`str`): |
There was a problem hiding this comment.
It's type should mention that it's optional. Also, I don't quite understand what I should do with kwargs here. I think an example would be useful to have.
src/accelerate/tracking.py
Outdated
| kwargs: | ||
| Additional key word arguments passed along to `dvclive.Live.log_metric()`. | ||
| """ | ||
| if step: |
There was a problem hiding this comment.
What if step == 0. Should it not still be set? I.e., would it be safer to check if step is not None?
dberenbaum
force-pushed
the
dvclive-callback
branch
from
a13d0be to
47c54c9
Compare
|
@BenjaminBossan Thanks for the catching those issues and the quick review! All should be resolved now. |
|
|
||
|
|
||
| @require_dvclive | ||
| @mock.patch("dvclive.live.get_dvc_repo", return_value=None) |
There was a problem hiding this comment.
@dberenbaum FYI we're seeing failures on the transformers side in the example scripts because of this. Is there a better way to ignore the git repo than patching for cases where we're running a full training script? (Like via an env variable or something). Otherwise I'll have to disable dvc for all of the example script tests in transformers which wouldn't be ideal. https://github.com/huggingface/accelerate/actions/runs/7023901505/job/19111477258
There was a problem hiding this comment.
I need to take a closer look when I have a minute, but you can use DVCLIVE_TEST = true to disable the git-related functionality. For example:
accelerate/tests/test_examples.py
Line 208 in 31c723f
There was a problem hiding this comment.
Ah I missed that one, fantastic! That should do the job just fine I think. I’ll get to that tommorow and cc you. Thanks!
There was a problem hiding this comment.
Is #2196 related?
There was a problem hiding this comment.
It’s not, we saw dependency errors in an isolated env related to the release
There was a problem hiding this comment.
Thanks. Added a comment there since it should be fixed now. Let me know if it would be helpful to submit a PR for either or if it's quicker to take care of it yourself.
There was a problem hiding this comment.
@dberenbaum a PR would be most welcome!
What does this PR do?
Adds a DVCLive tracker. DVCLive is a logger for DVC, which version data, models, metrics, etc. and keeps them connected to your Git repo. They can then be visualized in several different interfaces.
A PR was recently merged to add a logger to
transformers, and this PR adds support foraccelerate.Before submitting
Pull Request section?
to it if that's the case.
documentation guidelines, and
here are tips on formatting docstrings.
Who can review?
@muellerzr From what I can tell, it looks like you would be the relevant reviewer. Please feel free to redirect me otherwise.