Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: optuna tune script PoC #1411

Merged
merged 8 commits into from
Apr 17, 2022
Merged

feat: optuna tune script PoC #1411

merged 8 commits into from
Apr 17, 2022

Conversation

bagxi
Copy link
Member

@bagxi bagxi commented Mar 28, 2022

Pull Request FAQ

Description

Related Issue

Type of Change

  • Examples / docs / tutorials / contributors update
  • Bug fix (non-breaking change which fixes an issue)
  • Improvement (non-breaking change which improves an existing feature)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

PR review

Anyone in the community is free to review the PR once the tests have passed.
If we didn't discuss your PR in Github issues there's a high chance it will not be merged.

Checklist

  • Have you updated tests for the new functionality?
  • Have you added your new classes/functions to the docs?
  • Have you updated the CHANGELOG?
  • Have you run colab minimal CI/CD with latest requirements? Please attach the notebook link.
  • Have you run colab minimal CI/CD with minimal requirements? Please attach the notebook link.
  • Have you checked XLA integration? Please attach the notebook link.
  • Have you checked distributed XLA integration? Please attach the notebook link.

@bagxi bagxi requested a review from Scitator as a code owner March 28, 2022 16:20
@bagxi bagxi self-assigned this Mar 28, 2022
@bagxi bagxi requested a review from ditwoo as a code owner March 28, 2022 16:20
ditwoo
ditwoo previously approved these changes Apr 1, 2022
View reviewed changes
@Scitator
Copy link
Member

Scitator commented Apr 2, 2022

LGTM!
could you please:

  • update the changelog?
  • add catalyst-tune command run to CI/CD and XLA? so we could easily check them on GPU-based setup?

@mergify mergify bot dismissed ditwoo’s stale review April 7, 2022 18:56

Pull request has been modified.

@bagxi
Copy link
Member Author

bagxi commented Apr 7, 2022 •
edited
Loading

LGTM! could you please:

  • update the changelog?
  • add catalyst-tune command run to CI/CD and XLA? so we could easily check them on GPU-based setup?
  1. done
  2. CPU/GPU - https://github.com/catalyst-team/catalyst/blob/75c04d1535eaadf77368158407cbff6d402abcab/tests/contrib/scripts/test_tune.py#L45L51
    @Scitator , is there a way to check XLA support using pytests or XLA notebook should be updated?

@Scitator
Copy link
Member

@bagxi yeah, we have to update the notebook - this is XLA way :)

@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.

Powered by ReviewNB

@Scitator Scitator merged commit 7bd9eb5 into master Apr 17, 2022
@mergify mergify bot deleted the config_api_optuna branch April 17, 2022 10:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Scitator Scitator Scitator approved these changes

@ditwoo ditwoo ditwoo left review comments

Assignees

@bagxi bagxi

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@bagxi @Scitator @ditwoo