Fix type hints of tuner/batch_size_scaling.py #13518
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ar90n
force-pushed
the
fix-type-hints-tuner-batch_size_scaling.py
branch
from
aac9127 to
3f1d70c
Compare
ar90n
force-pushed
the
fix-type-hints-tuner-batch_size_scaling.py
branch
from
ec01aea to
ab9bcba
Compare
ar90n
requested review from
williamFalcon,
tchaton,
awaelchli,
Borda,
SeanNaren,
carmocca,
SkafteNicki,
justusschock,
kaushikb11 and
rohitgr7
as code owners
Borda
reviewed
Jul 4, 2022
awaelchli
reviewed
Jul 4, 2022
Co-authored-by: Adrian Wälchli <[email protected]>
for more information, see https://pre-commit.ci
To fix type check issue, add None check explicitly. This early return dons't change the behabior of _is_valid_batch_size. Because has_len_all_ranks always return True if dataloader is None.
mergify
bot
added
ready
mergify
bot
added
has conflicts
ready
…r-batch_size_scaling.py
auto-merge was automatically disabled
September 28, 2022 15:11
Head branch was pushed to by a user without write access
mergify
bot
added
ready
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
otaj
approved these changes
Sep 29, 2022
carmocca
reviewed
Sep 29, 2022
| @@ -31,6 +31,8 @@ | |||
| class BatchSizeFinder(Callback): | |||
| SUPPORTED_MODES = ("power", "binsearch") | |||
|
|
|||
| optimal_batch_size: Optional[int] | |||
There was a problem hiding this comment.
Why is this annotated in the class definition instead of in the __init__?
There was a problem hiding this comment.
@carmocca
This is my mistake. And my modification wasn't in time.
In the head of my fork branch, this annotation is moved into init.
May I create a new PR to move this annotation into init ?
12 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What does this PR do?
If wrong parameters are given as arguments,
_adjust_batch_sizecannot adjust batch size correctly. So this PR add some codes to raise ValueErrors in that cases.Fixes part of #13445
Does your PR introduce any breaking changes? If yes, please list them.
Before submitting
PR review
Anyone in the community is welcome to review the PR.
Before you start reviewing, make sure you have read the review guidelines. In short, see the following bullet-list:
Did you have fun?
Make sure you had fun coding 🙃