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

Refactor version checking into a utility #395

Merged
merged 8 commits into from
May 25, 2022
Merged

Refactor version checking into a utility #395

merged 8 commits into from
May 25, 2022

Conversation

muellerzr
Copy link
Collaborator

@muellerzr muellerzr commented May 25, 2022

Refactor version checking into a utility

What does this add?

This PR adds two new functions:

  • compare_versions
  • check_torch_version

Which checks a particular version, querying with a string of the comparitor (such as "<" or ">=" or "!=")

Why is it needed?

I was noticing we do a lot of checks with version.parse in the code, and we already had a versions submodule for getting the current torch version. As a result, it made sense to expand this utility further to have it check for all of our torch versions.

Also considering that most of them are comparing the torch version specifically, I added a check_torch_version which shorthands the query on the user.

What parts of the API does this impact?

User-facing:

Nothing

Internal structure:

Modifies all version checks to use compare_versions or check_torch_version

Basic Usage Example(s):

For checking a non-torch library:

from accelerate.utils import compare_versions
compare_versions("fastai", "<", "2.0.0")

For checking a torch version:

from accelerate.utils import check_torch_version
check_torch_version("<=", "1.10")

@muellerzr muellerzr added the enhancement New feature or request label May 25, 2022
@muellerzr muellerzr requested a review from sgugger May 25, 2022 16:00
@muellerzr muellerzr marked this pull request as draft May 25, 2022 16:04
Copy link
Collaborator

@sgugger sgugger left a comment

Choose a reason for hiding this comment

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

Nice refactor!

src/accelerate/accelerator.py Outdated Show resolved Hide resolved
src/accelerate/utils/versions.py Outdated Show resolved Hide resolved
@muellerzr muellerzr marked this pull request as ready for review May 25, 2022 16:06
@HuggingFaceDocBuilderDev
Copy link

HuggingFaceDocBuilderDev commented May 25, 2022

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

@muellerzr muellerzr requested a review from sgugger May 25, 2022 16:36
Copy link
Collaborator

@sgugger sgugger left a comment

Choose a reason for hiding this comment

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

Left some nits to the doc, but otherwise looks great! Thanks for working on this!

src/accelerate/utils/versions.py Outdated Show resolved Hide resolved
src/accelerate/utils/versions.py Outdated Show resolved Hide resolved
src/accelerate/utils/versions.py Outdated Show resolved Hide resolved
src/accelerate/utils/versions.py Outdated Show resolved Hide resolved
src/accelerate/utils/versions.py Outdated Show resolved Hide resolved
src/accelerate/utils/versions.py Outdated Show resolved Hide resolved
Co-authored-by: Sylvain Gugger <35901082+sgugger@users.noreply.github.com>
@muellerzr muellerzr merged commit f6ec266 into main May 25, 2022
@muellerzr muellerzr deleted the version_refactor branch May 25, 2022 18:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants