-
Notifications
You must be signed in to change notification settings - Fork 297
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
Change DerelativizeTransform to not use model predictions when use_raw_status_quo
is True
or when the status quo is infeasible.
#2036
Conversation
This pull request was exported from Phabricator. Differential Revision: D51690727 |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2036 +/- ##
=======================================
Coverage 94.52% 94.52%
=======================================
Files 460 460
Lines 44460 44466 +6
=======================================
+ Hits 42025 42031 +6
Misses 2435 2435 ☔ View full report in Codecov by Sentry. |
This pull request was exported from Phabricator. Differential Revision: D51690727 |
…aw_status_quo` is `True` or when the status quo is infeasible. (facebook#2036) Summary: Pull Request resolved: facebook#2036 The was previously ignoring `use_raw_status_quo` and was using the values predicted by the model unless this errored out. This is misleading since we shouldn't be using the model predictions when `use_raw_status_quo` is `True`. This also resulted in weird behavior in the case where the status quo arm was within the search space bounds but didn't satisfy the parameter constraints. This transform would then use the model to predict the metrics of the status quo (ignoring that `use_raw_status_quo` was `True`), but the model wasn't trained on the status quo since it didn't satisfy the constraints. Thus, this resulted in the model having to extrapolate and producing very weird predictions. This diff changes the behavior to only use the model to predict the status quo metrics when (1) `use_raw_status_quo` is `False` and (2) the status quo is actually feasible. Reviewed By: Balandat Differential Revision: D51690727 fbshipit-source-id: 472e0b9462f6d5efe5554dd740a64146ee198d8f
c4cead9
to
0b423df
Compare
…aw_status_quo` is `True` or when the status quo is infeasible. (facebook#2036) Summary: Pull Request resolved: facebook#2036 The was previously ignoring `use_raw_status_quo` and was using the values predicted by the model unless this errored out. This is misleading since we shouldn't be using the model predictions when `use_raw_status_quo` is `True`. This also resulted in weird behavior in the case where the status quo arm was within the search space bounds but didn't satisfy the parameter constraints. This transform would then use the model to predict the metrics of the status quo (ignoring that `use_raw_status_quo` was `True`), but the model wasn't trained on the status quo since it didn't satisfy the constraints. Thus, this resulted in the model having to extrapolate and producing very weird predictions. This diff changes the behavior to only use the model to predict the status quo metrics when (1) `use_raw_status_quo` is `False` and (2) the status quo is actually feasible. Reviewed By: Balandat Differential Revision: D51690727 fbshipit-source-id: 3f2648a0091c88e5e4c9c2f388421d3f563f6372
This pull request was exported from Phabricator. Differential Revision: D51690727 |
0b423df
to
470811f
Compare
This pull request has been merged in ef3b1c1. |
Summary:
The was previously ignoring
use_raw_status_quo
and was using the values predicted by the model unless this errored out. This is misleading since we shouldn't be using the model predictions whenuse_raw_status_quo
isTrue
.This also resulted in weird behavior in the case where the status quo arm was within the search space bounds but didn't satisfy the parameter constraints. This transform would then use the model to predict the metrics of the status quo (ignoring that
use_raw_status_quo
wasTrue
), but the model wasn't trained on the status quo since it didn't satisfy the constraints. Thus, this resulted in the model having to extrapolate and producing very weird predictions.This diff changes the behavior to only use the model to predict the status quo metrics when (1)
use_raw_status_quo
isFalse
and (2) the status quo is actually feasible.Reviewed By: Balandat
Differential Revision: D51690727