Fix mypy error about PyTorch Distributed#4413
Conversation
|
@cross32768 Could you review this PR since it follows up #4281. It will fix the error in https://github.com/optuna/optuna/actions/runs/4139175958/jobs/7156458918.
|
| ) | ||
|
|
||
| _g_pg: List[Optional["ProcessGroup"]] = [None] | ||
| _g_pg: Optional["ProcessGroup"] = None |
There was a problem hiding this comment.
This change seems reasonable to me since only the first element is used in the master code.
I'd like to use a local variable to manage the process group for Optuna in #4268, but I don't think we should work on it in this PR.
There was a problem hiding this comment.
Thank you for your follow-up.
cross32768
left a comment
There was a problem hiding this comment.
I checked this change in my local environment and confirmed that this change fix the mypy error. It looks reasonable fix. LGTM!
toshihikoyanase
left a comment
There was a problem hiding this comment.
I also confirmed that the change worked with the pytorch distributed example in oputna/optuna-examples. LGTM!
Motivation
Resolve #4116
Description of the changes
This PR fixes mypy error in
integration/pytorch_distributed.py, by making it explicit that__init__()ofTorchDistributedTrialchanges a global variable. Note that mypy error of the line was used to be ignored before PR #4281 but I removedtype: ignoreto reduce unused warnings. Mypy recognize this as error again from 1.0.0.