Skip to content

Conversation

@michaelriedl
Copy link

@michaelriedl michaelriedl commented Dec 1, 2024

Motivation

I wanted to bring the Optuna plugin up to date with the newest version. I made the necessary changes to make the plugin compatible. I also added in the GPSampler even though it is still experimental since some people may want to use it.

Have you read the Contributing Guidelines on pull requests?

Yes.

Test Plan

I modified the existing tests in the plugin folder. I have tested that they pass on my WSL setup.

Related Issues and PRs

Issues:
#2562
#2162

PRs:
#2360

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Dec 1, 2024
@michaelriedl
Copy link
Author

I did not realize that I basically duplicated #2360. Can we merge one of these in?

@michaelriedl
Copy link
Author

It also appears that the workflows for CI do not trigger with a PR from a forked branch.

@himkt
Copy link
Contributor

himkt commented Jan 23, 2025

Hello @jesszzzz, can we ask you to review the PR?

@himkt
Copy link
Contributor

himkt commented Feb 4, 2025

Let me kindly ping @jesszzzz.

@jesszzzz
Copy link
Contributor

jesszzzz commented Feb 4, 2025

Sorry for the delay! I do expect some tests will break from this, #3020 enables running CI on forked PRs so please rebase when that's merged. Otherwise these changes look fine to me!

@himkt
Copy link
Contributor

himkt commented Feb 4, 2025

Thank you for letting us know!
cc. @michaelriedl

@michaelriedl
Copy link
Author

michaelriedl commented Feb 5, 2025

Sorry for the delay! I do expect some tests will break from this, #3020 enables running CI on forked PRs so please rebase when that's merged. Otherwise these changes look fine to me!

@jesszzzz Rebased as requested.

@michaelriedl
Copy link
Author

@jesszzzz it looks like you need to approve the 2 workflows to run since I am a first time contributor. I ran all the Pytest tests locally without issue, just need to run the workflows.

@himkt
Copy link
Contributor

himkt commented Feb 25, 2025

Let me kindly ping as well. 🙏 @jesszzzz

@tadejsv
Copy link

tadejsv commented Mar 21, 2025

@jesszzzz any updates here?

@nefario7
Copy link

Any updates on getting this merged? Getting this in will be awesome
@jesszzzz

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants