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.

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

Implement log scale of parallel coordinate #627

Merged
merged 3 commits into from
Oct 2, 2023

Conversation

keisuke-umezawa
Copy link
Member

@keisuke-umezawa keisuke-umezawa commented Sep 24, 2023

Contributor License Agreement

This repository (optuna-dashboard) and Goptuna share common code.
This pull request may therefore be ported to Goptuna.
Make sure that you understand the consequences concerning licenses and check the box below if you accept the term before creating this pull request.

  • I agree this patch may be ported to Goptuna by other Goptuna contributors.

Reference Issues/PRs

NA

Log axis if distribution is log scale for parallel coordinate plot

scripts

import logging

import optuna
from optuna_dashboard._app import create_app


host = "127.0.0.1"
port = 8080

optuna.logging.set_verbosity(logging.CRITICAL)


def create_optuna_storage() -> optuna.storages.InMemoryStorage:
    storage = optuna.storages.InMemoryStorage()

    # Single-objective study
    study = optuna.create_study(study_name="single-objective", storage=storage)

    def objective_single(trial: optuna.Trial) -> float:
        x1 = trial.suggest_float("x1", 0, 10)
        x2 = trial.suggest_float("x2", 0.1, 10, log=True)
        x3 = trial.suggest_categorical("x3", ["foo", "bar"])
        return (x1 - 2) ** 2 + (x2 - 5) ** 2

    study.optimize(objective_single, n_trials=100)

    return storage


def main() -> None:
    storage = create_optuna_storage()
    app = create_app(storage, debug=True)
    app.run(host=host, port=port, reloader=True)


if __name__ == "__main__":
    main()

@codecov
Copy link

codecov bot commented Sep 24, 2023

Codecov Report

Merging #627 (4250c59) into main (02bfb79) will increase coverage by 6.04%.
Report is 21 commits behind head on main.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main     #627      +/-   ##
==========================================
+ Coverage   56.40%   62.45%   +6.04%     
==========================================
  Files          35       35              
  Lines        2193     2224      +31     
==========================================
+ Hits         1237     1389     +152     
+ Misses        956      835     -121     
Files Coverage Δ
optuna_dashboard/_preference_setting.py 88.88% <ø> (ø)

... and 4 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

)
const ticktext = tickvals.map((x) => `${Math.pow(10, x).toPrecision(3)}`)
return { logValues, tickvals, ticktext }
}
Copy link
Member Author

Choose a reason for hiding this comment

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

@c-bata c-bata self-assigned this Sep 25, 2023
@c-bata c-bata assigned not522 and gen740 and unassigned c-bata Sep 27, 2023
@c-bata
Copy link
Member

c-bata commented Sep 27, 2023

@not522 @gen740 Could you review this PR?

ticktext,
}
} else {
// numerical and non-log
Copy link
Member

Choose a reason for hiding this comment

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

A nit.

Suggested change
// numerical and non-log
// numerical and linear

Or you can simply remove these comments.

Copy link
Contributor

@gen740 gen740 left a comment

Choose a reason for hiding this comment

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

LGTM! It works as expected.

Copy link
Member

@not522 not522 left a comment

Choose a reason for hiding this comment

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

Thank you for your PR. Please check my comment.

})
const minValue = Math.min(...logValues)
const maxValue = Math.max(...logValues)
const range = [minValue, maxValue]
Copy link
Member

Choose a reason for hiding this comment

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

Both ends of ticks should be within the range.

Suggested change
const range = [minValue, maxValue]
const range = [Math.floor(minValue), Math.ceil(maxValue)]

@keisuke-umezawa
Copy link
Member Author

@not522 Thank you for reviewing. Could you check it again?

Copy link
Member

@not522 not522 left a comment

Choose a reason for hiding this comment

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

LGTM!

@not522 not522 merged commit d56170c into optuna:main Oct 2, 2023
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants