-
Notifications
You must be signed in to change notification settings - Fork 402
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
MetricTracker
use higher_is_better
as default for maximize
#2545
Comments
Hi! thanks for your contribution!, great first issue! |
Hi @MoustHolmes, thanks for the code suggestion :) |
MetricTracker
use higher_is_better
as default for maximize
🚀 Feature
Change the
maximize
argument for theMetricTracker
wrapper from defaulting toTrue
to using thehigher_is_better
property of the metric(s) if nomaximize
is supplied.Motivation
Reduce boilerplate code. In 99% of use cases, you want to track the best metric rather than the worst. Most metrics you would want to track already have a
higher_is_better
property, so manually typingmaximize
is just boilerplate. Additionally, the current approach of defaulting toTrue
is kinda arbitrary, as it's roughly 50/50 whether you want to maximize or minimize a random metric.Pitch
The amount of code changes needed is minimal as it only affects the
__init__
method, leaving the rest of the implementation unchanged.Here's a suggested implementation:
Additional context
I haven't made many pull requests before, so i'm very open to suggestions and changes :)
The text was updated successfully, but these errors were encountered: