Skip to content
This repository has been archived by the owner on Dec 1, 2021. It is now read-only.

Fix MAX_STEPS = 0 error by add suggestion and exit #1195

Merged
merged 5 commits into from
Sep 3, 2020

Conversation

oatawa1
Copy link
Contributor

@oatawa1 oatawa1 commented Sep 2, 2020

What this patch does to fix the issue.

Change the minimum no. of MAX_STEPS to 1.

Link to any relevant issues or pull requests.

#1194

@oatawa1 oatawa1 requested a review from iizukak September 2, 2020 06:02
@oatawa1 oatawa1 self-assigned this Sep 2, 2020
@blueoil-butler blueoil-butler bot added the CI: auto-run Run CI automatically label Sep 2, 2020
@bo-code-review-bot
Copy link

This PR needs Approvals as follows.

  • Ownership Approval for / from iizukak, tkng, ruimashita
  • Readability Approval for Python from tkng, tsawada, tfujiwar, a-hanamoto

Please choose reviewers and requet reviews!

Click to see how to approve each reviews

You can approve this PR by triggered comments as follows.

  • Approve all reviews requested to you (readability and ownership) and LGTM review
    Approval, LGTM

  • Approve all ownership reviews
    Ownership Approval or OA

  • Approve all readability reviews
    Readability Approval or RA

  • Approve specified review targets

    • Example of Ownership Reviewer of /: Ownership Approval for / or OA for /
    • Example of Readability Reviewer of Python: Readability Approval for Python or RA for Python
  • Approve LGTM review
    LGTM

See all trigger comments

Please replace [Target] to review target

  • Ownership Approval
    • Ownership Approval for [Target]
    • OA for [Target]
    • Ownership Approval
    • OA
    • Approval
  • Readability Approval
    • Readability Approval for [Target]
    • RA for [Target]
    • [Target] Readability Approval
    • [Target] RA
    • Readability Approval
    • RA
    • Approval
  • LGTM
    • LGTM
    • lgtm

Copy link
Member

@iizukak iizukak left a comment

Choose a reason for hiding this comment

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

OA

@oatawa1
Copy link
Contributor Author

oatawa1 commented Sep 3, 2020

@iizukak Thank you.

@oatawa1 oatawa1 requested a review from tfujiwar September 3, 2020 01:16
Copy link
Contributor

@tfujiwar tfujiwar left a comment

Choose a reason for hiding this comment

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

I left 1 question.

@oatawa1 oatawa1 changed the title Fix MAX_STEPS = 0 error Fix MAX_STEPS = 0 error by add suggestion and exit Sep 3, 2020
@oatawa1 oatawa1 requested a review from tfujiwar September 3, 2020 03:53
Copy link
Contributor

@tfujiwar tfujiwar left a comment

Choose a reason for hiding this comment

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

RA
with small suggestions.
Thank you!

@@ -192,8 +193,14 @@ def start_training(config, profile_step):
# Calculate max steps. The priority of config.MAX_EPOCHS is higher than config.MAX_STEPS.
if "MAX_EPOCHS" in config:
max_steps = int(train_dataset.num_per_epoch / config.BATCH_SIZE * config.MAX_EPOCHS)
if max_steps < 1:
print("The max_steps is less than 1, consider reduce BATCH_SIZE. exit.")
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
print("The max_steps is less than 1, consider reduce BATCH_SIZE. exit.")
print("The max_steps is less than 1, consider reduce BATCH_SIZE. exit.", file=sys.stderr)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

else:
max_steps = config.MAX_STEPS
if max_steps < 1:
print("The max_steps is less than 1, consider set MAX_STEPS greater than 0. exit.")
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
print("The max_steps is less than 1, consider set MAX_STEPS greater than 0. exit.")
print("The max_steps is less than 1, consider set MAX_STEPS greater than 0. exit.", file=sys.stderr)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

@oatawa1
Copy link
Contributor Author

oatawa1 commented Sep 3, 2020

@tfujiwar Thank you. I fixed your suggestion. I will merge this after it passed CI.

@iizukak
Copy link
Member

iizukak commented Sep 3, 2020

/ready

@bo-mergebot
Copy link
Contributor

bo-mergebot bot commented Sep 3, 2020

⏳Merge job is queued...

@bo-mergebot bo-mergebot bot merged commit 2b80ed1 into blue-oil:master Sep 3, 2020
@oatawa1 oatawa1 deleted the fix-max-steps-0 branch September 3, 2020 05:05
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
CI: auto-run Run CI automatically
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants