Skip to content

Add PyTorch integration#62

Merged
contramundum53 merged 20 commits intooptuna:mainfrom
nabenabe0928:code-fix/add-pytorch
Feb 14, 2024
Merged

Add PyTorch integration#62
contramundum53 merged 20 commits intooptuna:mainfrom
nabenabe0928:code-fix/add-pytorch

Conversation

@nabenabe0928
Copy link
Copy Markdown
Contributor

@nabenabe0928 nabenabe0928 commented Jan 29, 2024

Motivation

Description of the changes

y0z
y0z reviewed Feb 5, 2024
View reviewed changes
Copy link
Copy Markdown
Member

@y0z y0z left a comment

Choose a reason for hiding this comment

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

I leave some comments.

pyproject.toml Outdated
"tensorboard",
"tensorflow",
"torch",
"torchaudio",
Copy link
Copy Markdown
Member

@y0z y0z Feb 5, 2024

Choose a reason for hiding this comment

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

Are toachaudio and torchvision actually required? (cf. the doc)

@y0z
Copy link
Copy Markdown
Member

y0z commented Feb 5, 2024

This PR progresses optuna/optuna#4484.
Related PR: optuna/optuna#5213.

@y0z y0z mentioned this pull request Feb 5, 2024
Merged
@contramundum53
Copy link
Copy Markdown
Member

contramundum53 commented Feb 6, 2024

@nabenabe0928 Could you add this CI?

y0z
y0z reviewed Feb 6, 2024
View reviewed changes
Copy link
Copy Markdown
Member

@y0z y0z left a comment •
edited
Loading

Choose a reason for hiding this comment

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

I suggest the following changes.

Also, please add PyTorch integrations to init.py as same as other integrations.

@codecov
Copy link
Copy Markdown

codecov bot commented Feb 6, 2024 •
edited
Loading

Codecov Report

Attention: 33 lines in your changes are missing coverage. Please review.

Comparison is base (33a2d2d) 64.59% compared to head (a74c4e0) 67.44%.

Files Patch % Lines
optuna_integration/pytorch_distributed.py 88.46% 24 Missing ⚠️
optuna_integration/pytorch_lightning.py 92.40% 6 Missing ⚠️
optuna_integration/__init__.py 0.00% 3 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##             main      #62      +/-   ##
==========================================
+ Coverage   64.59%   67.44%   +2.84%     
==========================================
  Files          29       32       +3     
  Lines        2409     2715     +306     
==========================================
+ Hits         1556     1831     +275     
- Misses        853      884      +31

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

y0z
y0z approved these changes Feb 7, 2024
View reviewed changes
Copy link
Copy Markdown
Member

@y0z y0z left a comment

Choose a reason for hiding this comment

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

LGTM

@y0z y0z removed their assignment Feb 7, 2024
@nabenabe0928
Copy link
Copy Markdown
Contributor Author

nabenabe0928 commented Feb 7, 2024 •
edited
Loading

@contramundum53 Could you merge this PR once you confirm it looks good to you?

contramundum53
contramundum53 approved these changes Feb 14, 2024
View reviewed changes
Copy link
Copy Markdown
Member

@contramundum53 contramundum53 left a comment

Choose a reason for hiding this comment

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

LGTM!

@contramundum53 contramundum53 merged commit 63b4c58 into optuna:main Feb 14, 2024
@nabenabe0928 nabenabe0928 added the feature Change that does not break compatibility, but affects the public interfaces. label Feb 14, 2024
@nabenabe0928 nabenabe0928 added this to the v3.6.0 milestone Feb 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@contramundum53 contramundum53 contramundum53 approved these changes

@y0z y0z y0z approved these changes

Assignees

No one assigned

Labels

feature Change that does not break compatibility, but affects the public interfaces.

Projects

None yet

Milestone

v3.6.0

Development

Successfully merging this pull request may close these issues.

3 participants

@nabenabe0928 @y0z @contramundum53