Skip to content

Fix mypy error about pytorch_distributed.py in Checks (integration)#4281

Merged
toshihikoyanase merged 1 commit intooptuna:masterfrom
Alnusjaponica:fix-checks-integration-pytorch-distributed
Jan 16, 2023
Merged

Fix mypy error about pytorch_distributed.py in Checks (integration)#4281
toshihikoyanase merged 1 commit intooptuna:masterfrom
Alnusjaponica:fix-checks-integration-pytorch-distributed

Conversation

@Alnusjaponica
Copy link
Copy Markdown
Contributor

@Alnusjaponica Alnusjaponica commented Dec 23, 2022

Motivation

Solve scheduled Checks (Integration) failure

Description of the changes

Currently, scheduled Checks (Integration) fails die to mypy check with --warn-unused-ignores option. This PR removes a part of the cause related to pytorch_distributed.py.
Although test for Checks (Integration) does't run when PR in opened, I confirmed this PR together with other "Fix mypy error" series reduces mypy error at CI on my fork.

@github-actions github-actions bot added the optuna.integration Related to the `optuna.integration` submodule. This is automatically labeled by github-actions. label Dec 23, 2022
@codecov-commenter
Copy link
Copy Markdown

Codecov Report

Merging #4281 (6c5b6c8) into master (b4cc098) will increase coverage by 0.01%.
The diff coverage is 11.11%.

@@            Coverage Diff             @@
##           master    #4281      +/-   ##
==========================================
+ Coverage   89.76%   89.78%   +0.01%     
==========================================
  Files         170      170              
  Lines       13261    13262       +1     
==========================================
+ Hits        11904    11907       +3     
+ Misses       1357     1355       -2     
Impacted Files Coverage Δ
optuna/integration/pytorch_distributed.py 36.50% <11.11%> (+0.33%) ⬆️
optuna/study/study.py 94.96% <0.00%> (+0.77%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@Alnusjaponica Alnusjaponica changed the title Fix mypy error abut pytorch_distributed.py in Checks (integration) Fix mypy error abut pytorch_distributed.py in Checks (integration) Dec 23, 2022
@Alnusjaponica Alnusjaponica marked this pull request as ready for review December 23, 2022 08:06
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Jan 1, 2023

This pull request has not seen any recent activity.

@github-actions github-actions bot added the stale Exempt from stale bot labeling. label Jan 1, 2023
@HideakiImamura
Copy link
Copy Markdown
Member

@toshihikoyanase @cross32768 Could you review this PR?

@github-actions github-actions bot removed the stale Exempt from stale bot labeling. label Jan 4, 2023
Copy link
Copy Markdown
Contributor

@cross32768 cross32768 left a comment

Choose a reason for hiding this comment

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

LGTM! I confirmed that 6 errors are appeared in master branch as shown below, and this PR resolves all.

optuna/integration/pytorch_distributed.py:37: error: Name "torch.distributed.ProcessGroup" is not defined  [name-defined]
optuna/integration/pytorch_distributed.py:117: error: Name "torch.distributed.ProcessGroup" is not defined  [name-defined]
optuna/integration/pytorch_distributed.py:122: error: Name "torch.distributed.ProcessGroup" is not defined  [name-defined]
optuna/integration/pytorch_distributed.py:127: error: Name "torch.distributed.ProcessGroup" is not defined  [name-defined]
optuna/integration/pytorch_distributed.py:128: error: Call to untyped function "get_backend" in typed context  [no-untyped-call]
optuna/integration/pytorch_distributed.py:129: error: Name "torch.distributed.ProcessGroup" is not defined  [name-defined]

@github-actions
Copy link
Copy Markdown
Contributor

This pull request has not seen any recent activity.

@github-actions github-actions bot added the stale Exempt from stale bot labeling. label Jan 12, 2023
@Alnusjaponica
Copy link
Copy Markdown
Contributor Author

Thank you for your approval. @cross32768
Could you take a look at the change? @toshihikoyanase

@github-actions github-actions bot removed the stale Exempt from stale bot labeling. label Jan 15, 2023
@toshihikoyanase toshihikoyanase added the CI Continuous integration. label Jan 16, 2023
Copy link
Copy Markdown
Member

@toshihikoyanase toshihikoyanase left a comment

Choose a reason for hiding this comment

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

I'm sorry for the delayed response. I confirmed that the errors were resolved by this PR on my local PC. Thank you!

@toshihikoyanase toshihikoyanase added this to the v3.1.0 milestone Jan 16, 2023
@toshihikoyanase toshihikoyanase merged commit 48fec20 into optuna:master Jan 16, 2023
@Alnusjaponica Alnusjaponica deleted the fix-checks-integration-pytorch-distributed branch January 16, 2023 06:34
@c-bata c-bata changed the title Fix mypy error abut pytorch_distributed.py in Checks (integration) Fix mypy error about pytorch_distributed.py in Checks (integration) Jan 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CI Continuous integration. optuna.integration Related to the `optuna.integration` submodule. This is automatically labeled by github-actions.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants