Skip to content
This repository has been archived by the owner on Mar 12, 2024. It is now read-only.

Optuna Integration #215

Merged
merged 4 commits into from
Oct 21, 2021
Merged

Optuna Integration #215

merged 4 commits into from
Oct 21, 2021

Conversation

GoldenCorgi
Copy link
Contributor

Hey @danthe3rd !

This should work for the integration mentioned in #145

Two key things I'm not too sure about (sorry again! pretty bad at open source contribution)

  1.  ```def from_optuna(study: "optuna.study.Study") -> "Experiment":  ```
    

Is the type checking / type hinting here correct?
2. I added optuna as a dependency for dev & import it during the testing, is this correct?

If everything works out - I'm thinking of implementing this to the other popular hyperparam tuning libraries as well - like HyperOpt & Ray Tune

Thanks!

@danthe3rd
Copy link
Contributor

Hey @GoldenCorgi - nice to see you contributing again :)

Is the type checking / type hinting here correct?

It looks good to me, let's see what the CI say about it

  1. I added optuna as a dependency for dev & import it during the testing, is this correct?

Yes perfect. Indeed we don't want to add a dependency for end-users, but it's fine to add dev dependencies

I'll merge it when the CI is all green.

Comment on lines 45 to 47
def objective(trial):
x = trial.suggest_float("x", -1, 1)
return x ** 2
Copy link
Contributor

@danthe3rd danthe3rd Oct 15, 2021

Choose a reason for hiding this comment

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

Missing some typing here - let's not worry too much about tests typing tho, this should work

Suggested change
def objective(trial):
x = trial.suggest_float("x", -1, 1)
return x ** 2
def objective(trial: tp.Any) -> float:
x = trial.suggest_float("x", -1, 1)
return x ** 2

@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 Oct 15, 2021
@GoldenCorgi
Copy link
Contributor Author

Updated this in the latest commit!

Missing some typing here - let's not worry too much about tests typing tho, this should work

def test_from_optuna() -> None:
def objective(trial: "optuna.trial.Trial") -> float:
x = trial.suggest_float("x", -1, 1)
return x ** 2
study = optuna.create_study()
study.optimize(objective, n_trials=3)
# Create a dataframe from the study.

@danthe3rd danthe3rd merged commit 79b3d52 into facebookresearch:main Oct 21, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
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.

3 participants