Inference trace and Best Point Recommendation (BPR) bugfix #4128
+376
−457
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
meta-cla
bot
added
the
CLA Signed
Contributor
|
This pull request was exported from Phabricator. Differential Revision: D80019803 |
hvarfner
force-pushed
the
export-D80019803
branch
from
d0f646c to
fb533d6
Compare
Contributor
|
This pull request was exported from Phabricator. Differential Revision: D80019803 |
hvarfner
pushed a commit
to hvarfner/Ax
that referenced
this pull request
Aug 12, 2025
…4128) Summary: This diff addresses two issues in the computation of inference trace: 1. The generation strategy is copied inside run_optimization_with_orchestrator --> we retrieve the traces on an unused generation strategy --> get_best_point defaults to the best raw observation on ALL obserations 2. Relevant data not filtered in the fallback option for get_best_parameters_from_model_predictions_with_trial_index Both of these individually lead to the inference trace being incorrect - the first to the best raw value of ALL trials, the second to the best predicted across ALL trials. Changes: - (re?)-Moved copying of generation strategy - Added argument use_model_only_if_good to force model-based BPR even if model fit is bad - Clearer sequencing in get_best_parameters_from_model_predictions_with_trial_index Differential Revision: D80019803
hvarfner
force-pushed
the
export-D80019803
branch
from
fb533d6 to
bd06470
Compare
Contributor
|
This pull request was exported from Phabricator. Differential Revision: D80019803 |
hvarfner
pushed a commit
to hvarfner/Ax
that referenced
this pull request
Aug 12, 2025
…4128) Summary: This diff addresses two issues in the computation of inference trace: 1. The generation strategy is copied inside run_optimization_with_orchestrator --> we retrieve the traces on an unused generation strategy --> get_best_point defaults to the best raw observation on ALL obserations 2. Relevant data not filtered in the fallback option for get_best_parameters_from_model_predictions_with_trial_index Both of these individually lead to the inference trace being incorrect - the first to the best raw value of ALL trials, the second to the best predicted across ALL trials. Changes: - (re?)-Moved copying of generation strategy - Added argument use_model_only_if_good to force model-based BPR even if model fit is bad - Clearer sequencing in get_best_parameters_from_model_predictions_with_trial_index Differential Revision: D80019803
hvarfner
force-pushed
the
export-D80019803
branch
from
bd06470 to
8743553
Compare
Contributor
|
This pull request was exported from Phabricator. Differential Revision: D80019803 |
hvarfner
pushed a commit
to hvarfner/Ax
that referenced
this pull request
Aug 12, 2025
…4128) Summary: This diff addresses two issues in the computation of inference trace: 1. The generation strategy is copied inside run_optimization_with_orchestrator --> we retrieve the traces on an unused generation strategy --> get_best_point defaults to the best raw observation on ALL obserations 2. Relevant data not filtered in the fallback option for get_best_parameters_from_model_predictions_with_trial_index Both of these individually lead to the inference trace being incorrect - the first to the best raw value of ALL trials, the second to the best predicted across ALL trials. Changes: - (re?)-Moved copying of generation strategy - Added argument use_model_only_if_good to force model-based BPR even if model fit is bad - Clearer sequencing in get_best_parameters_from_model_predictions_with_trial_index Differential Revision: D80019803
hvarfner
force-pushed
the
export-D80019803
branch
from
8743553 to
72b859b
Compare
Contributor
|
This pull request was exported from Phabricator. Differential Revision: D80019803 |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4128 +/- ##
=======================================
Coverage 96.72% 96.73%
=======================================
Files 582 582
Lines 60742 60738 -4
=======================================
Hits 58754 58754
+ Misses 1988 1984 -4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
hvarfner
pushed a commit
to hvarfner/Ax
that referenced
this pull request
Aug 12, 2025
…4128) Summary: This diff addresses two issues in the computation of inference trace: 1. The generation strategy is copied inside run_optimization_with_orchestrator --> we retrieve the traces on an unused generation strategy --> get_best_point defaults to the best raw observation on ALL obserations 2. Relevant data not filtered in the fallback option for get_best_parameters_from_model_predictions_with_trial_index Both of these individually lead to the inference trace being incorrect - the first to the best raw value of ALL trials, the second to the best predicted across ALL trials. Changes: - (re?)-Moved copying of generation strategy - Added argument use_model_only_if_good to force model-based BPR even if model fit is bad - Clearer sequencing in get_best_parameters_from_model_predictions_with_trial_index Differential Revision: D80019803
hvarfner
force-pushed
the
export-D80019803
branch
from
72b859b to
fe2fc61
Compare
Contributor
|
This pull request was exported from Phabricator. Differential Revision: D80019803 |
hvarfner
force-pushed
the
export-D80019803
branch
from
fe2fc61 to
f162393
Compare
hvarfner
pushed a commit
to hvarfner/Ax
that referenced
this pull request
Aug 13, 2025
…4128) Summary: This diff addresses two issues in the computation of inference trace: 1. The generation strategy is copied inside run_optimization_with_orchestrator --> we retrieve the traces on an unused generation strategy --> get_best_point defaults to the best raw observation on ALL obserations 2. Relevant data not filtered in the fallback option for get_best_parameters_from_model_predictions_with_trial_index Both of these individually lead to the inference trace being incorrect - the first to the best raw value of ALL trials, the second to the best predicted across ALL trials. Changes: - Moved copying of generation strategy to the level`benchmark_replication`, since results need to be computed on the used `generation_strategy` and not an empty copy. This means that `run_optimization_with_orchestrator` no longer `clone_and_reset`'s the GS. - Clearer sequencing in get_best_parameters_from_model_predictions_with_trial_index - Removed model fit quality check as part of BPR Previous, redacted changes: - Added argument use_model_only_if_good to force model-based BPR even if model fit is bad Differential Revision: D80019803
Contributor
|
This pull request was exported from Phabricator. Differential Revision: D80019803 |
hvarfner
pushed a commit
to hvarfner/Ax
that referenced
this pull request
Aug 13, 2025
…4128) Summary: This diff addresses two issues in the computation of inference trace: 1. The generation strategy is copied inside run_optimization_with_orchestrator --> we retrieve the traces on an unused generation strategy --> get_best_point defaults to the best raw observation on ALL obserations 2. Relevant data not filtered in the fallback option for get_best_parameters_from_model_predictions_with_trial_index Both of these individually lead to the inference trace being incorrect - the first to the best raw value of ALL trials, the second to the best predicted across ALL trials. Changes: - Moved copying of generation strategy to the level`benchmark_replication`, since results need to be computed on the used `generation_strategy` and not an empty copy. This means that `run_optimization_with_orchestrator` no longer `clone_and_reset`'s the GS. - Clearer sequencing in get_best_parameters_from_model_predictions_with_trial_index - Removed model fit quality check as part of BPR Previous, redacted changes: - Added argument use_model_only_if_good to force model-based BPR even if model fit is bad Differential Revision: D80019803
hvarfner
force-pushed
the
export-D80019803
branch
from
f162393 to
50ee506
Compare
Contributor
|
This pull request was exported from Phabricator. Differential Revision: D80019803 |
hvarfner
force-pushed
the
export-D80019803
branch
from
50ee506 to
f3c1203
Compare
hvarfner
pushed a commit
to hvarfner/Ax
that referenced
this pull request
Aug 14, 2025
…4128) Summary: This diff addresses two issues in the computation of inference trace: 1. The generation strategy is copied inside run_optimization_with_orchestrator --> we retrieve the traces on an unused generation strategy --> get_best_point defaults to the best raw observation on ALL obserations 2. Relevant data not filtered in the fallback option for get_best_parameters_from_model_predictions_with_trial_index Both of these individually lead to the inference trace being incorrect - the first to the best raw value of ALL trials, the second to the best predicted across ALL trials. Changes: - Moved copying of generation strategy to the level`benchmark_replication`, since results need to be computed on the used `generation_strategy` and not an empty copy. This means that `run_optimization_with_orchestrator` no longer `clone_and_reset`'s the GS. - Clearer sequencing in get_best_parameters_from_model_predictions_with_trial_index - Removed model fit quality check as part of BPR Previous, redacted changes: - Added argument use_model_only_if_good to force model-based BPR even if model fit is bad Differential Revision: D80019803
Contributor
|
This pull request was exported from Phabricator. Differential Revision: D80019803 |
hvarfner
pushed a commit
to hvarfner/Ax
that referenced
this pull request
Aug 14, 2025
…4128) Summary: This diff addresses two issues in the computation of inference trace: 1. The generation strategy is copied inside run_optimization_with_orchestrator --> we retrieve the traces on an unused generation strategy --> get_best_point defaults to the best raw observation on ALL obserations 2. Relevant data not filtered in the fallback option for get_best_parameters_from_model_predictions_with_trial_index Both of these individually lead to the inference trace being incorrect - the first to the best raw value of ALL trials, the second to the best predicted across ALL trials. Changes: - Moved copying of generation strategy to the level`benchmark_replication`, since results need to be computed on the used `generation_strategy` and not an empty copy. This means that `run_optimization_with_orchestrator` no longer `clone_and_reset`'s the GS. - Clearer sequencing in get_best_parameters_from_model_predictions_with_trial_index - Removed model fit quality check as part of BPR Previous, redacted changes: - Added argument use_model_only_if_good to force model-based BPR even if model fit is bad Differential Revision: D80019803
hvarfner
force-pushed
the
export-D80019803
branch
from
f3c1203 to
54462fc
Compare
Contributor
|
This pull request was exported from Phabricator. Differential Revision: D80019803 |
hvarfner
pushed a commit
to hvarfner/Ax
that referenced
this pull request
Aug 18, 2025
…4128) Summary: This diff addresses two issues in the computation of inference trace: 1. The generation strategy is copied inside run_optimization_with_orchestrator --> we retrieve the traces on an unused generation strategy --> get_best_point defaults to the best raw observation on ALL obserations 2. Relevant data not filtered in the fallback option for get_best_parameters_from_model_predictions_with_trial_index Both of these individually lead to the inference trace being incorrect - the first to the best raw value of ALL trials, the second to the best predicted across ALL trials. Changes: - Moved copying of generation strategy to the level`benchmark_replication`, since results need to be computed on the used `generation_strategy` and not an empty copy. This means that `run_optimization_with_orchestrator` no longer `clone_and_reset`'s the GS. - Clearer sequencing in get_best_parameters_from_model_predictions_with_trial_index - Removed model fit quality check as part of BPR Previous, redacted changes: - Added argument use_model_only_if_good to force model-based BPR even if model fit is bad Differential Revision: D80019803
Contributor
|
This pull request was exported from Phabricator. Differential Revision: D80019803 |
hvarfner
force-pushed
the
export-D80019803
branch
from
2757dee to
94cc283
Compare
hvarfner
pushed a commit
to hvarfner/Ax
that referenced
this pull request
Aug 18, 2025
…4128) Summary: Pull Request resolved: facebook#4128 This diff addresses two issues in the computation of inference trace: 1. The generation strategy is copied inside run_optimization_with_orchestrator --> we retrieve the traces on an unused generation strategy --> get_best_point defaults to the best raw observation on ALL obserations 2. Relevant data not filtered in the fallback option for get_best_parameters_from_model_predictions_with_trial_index Both of these individually lead to the inference trace being incorrect - the first to the best raw value of ALL trials, the second to the best predicted across ALL trials. Changes: - Moved copying of generation strategy to the level`benchmark_replication`, since results need to be computed on the used `generation_strategy` and not an empty copy. This means that `run_optimization_with_orchestrator` no longer `clone_and_reset`'s the GS. - Clearer sequencing in get_best_parameters_from_model_predictions_with_trial_index - Removed model fit quality check as part of BPR Previous, redacted changes: - Added argument use_model_only_if_good to force model-based BPR even if model fit is bad Differential Revision: D80019803
hvarfner
pushed a commit
to hvarfner/Ax
that referenced
this pull request
Aug 18, 2025
…4128) Summary: This diff addresses two issues in the computation of inference trace: 1. The generation strategy is copied inside run_optimization_with_orchestrator --> we retrieve the traces on an unused generation strategy --> get_best_point defaults to the best raw observation on ALL obserations 2. Relevant data not filtered in the fallback option for get_best_parameters_from_model_predictions_with_trial_index Both of these individually lead to the inference trace being incorrect - the first to the best raw value of ALL trials, the second to the best predicted across ALL trials. Changes: - Moved copying of generation strategy to the level`benchmark_replication`, since results need to be computed on the used `generation_strategy` and not an empty copy. This means that `run_optimization_with_orchestrator` no longer `clone_and_reset`'s the GS. - Clearer sequencing in get_best_parameters_from_model_predictions_with_trial_index - Removed model fit quality check as part of BPR Previous, redacted changes: - Added argument use_model_only_if_good to force model-based BPR even if model fit is bad Differential Revision: D80019803
hvarfner
pushed a commit
to hvarfner/Ax
that referenced
this pull request
Aug 18, 2025
…4128) Summary: This diff addresses two issues in the computation of inference trace: 1. The generation strategy is copied inside run_optimization_with_orchestrator --> we retrieve the traces on an unused generation strategy --> get_best_point defaults to the best raw observation on ALL obserations 2. Relevant data not filtered in the fallback option for get_best_parameters_from_model_predictions_with_trial_index Both of these individually lead to the inference trace being incorrect - the first to the best raw value of ALL trials, the second to the best predicted across ALL trials. Changes: - Moved copying of generation strategy to the level`benchmark_replication`, since results need to be computed on the used `generation_strategy` and not an empty copy. This means that `run_optimization_with_orchestrator` no longer `clone_and_reset`'s the GS. - Clearer sequencing in get_best_parameters_from_model_predictions_with_trial_index - Removed model fit quality check as part of BPR Previous, redacted changes: - Added argument use_model_only_if_good to force model-based BPR even if model fit is bad Differential Revision: D80019803
hvarfner
pushed a commit
to hvarfner/Ax
that referenced
this pull request
Aug 18, 2025
…4128) Summary: This diff addresses two issues in the computation of inference trace: 1. The generation strategy is copied inside run_optimization_with_orchestrator --> we retrieve the traces on an unused generation strategy --> get_best_point defaults to the best raw observation on ALL obserations 2. Relevant data not filtered in the fallback option for get_best_parameters_from_model_predictions_with_trial_index Both of these individually lead to the inference trace being incorrect - the first to the best raw value of ALL trials, the second to the best predicted across ALL trials. Changes: - Moved copying of generation strategy to the level`benchmark_replication`, since results need to be computed on the used `generation_strategy` and not an empty copy. This means that `run_optimization_with_orchestrator` no longer `clone_and_reset`'s the GS. - Clearer sequencing in get_best_parameters_from_model_predictions_with_trial_index - Removed model fit quality check as part of BPR Previous, redacted changes: - Added argument use_model_only_if_good to force model-based BPR even if model fit is bad Differential Revision: D80019803
hvarfner
pushed a commit
to hvarfner/Ax
that referenced
this pull request
Aug 18, 2025
…4128) Summary: This diff addresses two issues in the computation of inference trace: 1. The generation strategy is copied inside run_optimization_with_orchestrator --> we retrieve the traces on an unused generation strategy --> get_best_point defaults to the best raw observation on ALL obserations 2. Relevant data not filtered in the fallback option for get_best_parameters_from_model_predictions_with_trial_index Both of these individually lead to the inference trace being incorrect - the first to the best raw value of ALL trials, the second to the best predicted across ALL trials. Changes: - Moved copying of generation strategy to the level`benchmark_replication`, since results need to be computed on the used `generation_strategy` and not an empty copy. This means that `run_optimization_with_orchestrator` no longer `clone_and_reset`'s the GS. - Clearer sequencing in get_best_parameters_from_model_predictions_with_trial_index - Removed model fit quality check as part of BPR Previous, redacted changes: - Added argument use_model_only_if_good to force model-based BPR even if model fit is bad Differential Revision: D80019803
hvarfner
pushed a commit
to hvarfner/Ax
that referenced
this pull request
Aug 18, 2025
…4128) Summary: This diff addresses two issues in the computation of inference trace: 1. The generation strategy is copied inside run_optimization_with_orchestrator --> we retrieve the traces on an unused generation strategy --> get_best_point defaults to the best raw observation on ALL obserations 2. Relevant data not filtered in the fallback option for get_best_parameters_from_model_predictions_with_trial_index Both of these individually lead to the inference trace being incorrect - the first to the best raw value of ALL trials, the second to the best predicted across ALL trials. Changes: - Moved copying of generation strategy to the level`benchmark_replication`, since results need to be computed on the used `generation_strategy` and not an empty copy. This means that `run_optimization_with_orchestrator` no longer `clone_and_reset`'s the GS. - Clearer sequencing in get_best_parameters_from_model_predictions_with_trial_index - Removed model fit quality check as part of BPR Previous, redacted changes: - Added argument use_model_only_if_good to force model-based BPR even if model fit is bad Differential Revision: D80019803
hvarfner
force-pushed
the
export-D80019803
branch
from
94cc283 to
6528cdd
Compare
Contributor
|
This pull request was exported from Phabricator. Differential Revision: D80019803 |
hvarfner
pushed a commit
to hvarfner/Ax
that referenced
this pull request
Aug 18, 2025
…4128) Summary: This diff addresses two issues in the computation of inference trace: 1. The generation strategy is copied inside run_optimization_with_orchestrator --> we retrieve the traces on an unused generation strategy --> get_best_point defaults to the best raw observation on ALL obserations 2. Relevant data not filtered in the fallback option for get_best_parameters_from_model_predictions_with_trial_index Both of these individually lead to the inference trace being incorrect - the first to the best raw value of ALL trials, the second to the best predicted across ALL trials. Changes: - Moved copying of generation strategy to the level`benchmark_replication`, since results need to be computed on the used `generation_strategy` and not an empty copy. This means that `run_optimization_with_orchestrator` no longer `clone_and_reset`'s the GS. - Clearer sequencing in get_best_parameters_from_model_predictions_with_trial_index - Removed model fit quality check as part of BPR Previous, redacted changes: - Added argument use_model_only_if_good to force model-based BPR even if model fit is bad Differential Revision: D80019803
hvarfner
pushed a commit
to hvarfner/Ax
that referenced
this pull request
Aug 18, 2025
…4128) Summary: This diff addresses two issues in the computation of inference trace: 1. The generation strategy is copied inside run_optimization_with_orchestrator --> we retrieve the traces on an unused generation strategy --> get_best_point defaults to the best raw observation on ALL obserations 2. Relevant data not filtered in the fallback option for get_best_parameters_from_model_predictions_with_trial_index Both of these individually lead to the inference trace being incorrect - the first to the best raw value of ALL trials, the second to the best predicted across ALL trials. Changes: - Moved copying of generation strategy to the level`benchmark_replication`, since results need to be computed on the used `generation_strategy` and not an empty copy. This means that `run_optimization_with_orchestrator` no longer `clone_and_reset`'s the GS. - Clearer sequencing in get_best_parameters_from_model_predictions_with_trial_index - Removed model fit quality check as part of BPR Previous, redacted changes: - Added argument use_model_only_if_good to force model-based BPR even if model fit is bad Differential Revision: D80019803
hvarfner
pushed a commit
to hvarfner/Ax
that referenced
this pull request
Aug 19, 2025
…4128) Summary: This diff addresses two issues in the computation of inference trace: 1. The generation strategy is copied inside run_optimization_with_orchestrator --> we retrieve the traces on an unused generation strategy --> get_best_point defaults to the best raw observation on ALL obserations 2. Relevant data not filtered in the fallback option for get_best_parameters_from_model_predictions_with_trial_index Both of these individually lead to the inference trace being incorrect - the first to the best raw value of ALL trials, the second to the best predicted across ALL trials. Changes: - Moved copying of generation strategy to the level`benchmark_replication`, since results need to be computed on the used `generation_strategy` and not an empty copy. This means that `run_optimization_with_orchestrator` no longer `clone_and_reset`'s the GS. - Clearer sequencing in get_best_parameters_from_model_predictions_with_trial_index - Removed model fit quality check as part of BPR Previous, redacted changes: - Added argument use_model_only_if_good to force model-based BPR even if model fit is bad Differential Revision: D80019803
hvarfner
pushed a commit
to hvarfner/Ax
that referenced
this pull request
Aug 19, 2025
…4128) Summary: Pull Request resolved: facebook#4128 This diff addresses two issues in the computation of inference trace: 1. The generation strategy is copied inside run_optimization_with_orchestrator --> we retrieve the traces on an unused generation strategy --> get_best_point defaults to the best raw observation on ALL obserations 2. Relevant data not filtered in the fallback option for get_best_parameters_from_model_predictions_with_trial_index Both of these individually lead to the inference trace being incorrect - the first to the best raw value of ALL trials, the second to the best predicted across ALL trials. Changes: - Moved copying of generation strategy to the level`benchmark_replication`, since results need to be computed on the used `generation_strategy` and not an empty copy. This means that `run_optimization_with_orchestrator` no longer `clone_and_reset`'s the GS. - Clearer sequencing in get_best_parameters_from_model_predictions_with_trial_index - Removed model fit quality check as part of BPR Previous, redacted changes: - Added argument use_model_only_if_good to force model-based BPR even if model fit is bad Differential Revision: D80019803
hvarfner
pushed a commit
to hvarfner/Ax
that referenced
this pull request
Aug 19, 2025
…4128) Summary: This diff addresses two issues in the computation of inference trace: 1. The generation strategy is copied inside run_optimization_with_orchestrator --> we retrieve the traces on an unused generation strategy --> get_best_point defaults to the best raw observation on ALL obserations 2. Relevant data not filtered in the fallback option for get_best_parameters_from_model_predictions_with_trial_index Both of these individually lead to the inference trace being incorrect - the first to the best raw value of ALL trials, the second to the best predicted across ALL trials. Changes: - Moved copying of generation strategy to the level`benchmark_replication`, since results need to be computed on the used `generation_strategy` and not an empty copy. This means that `run_optimization_with_orchestrator` no longer `clone_and_reset`'s the GS. - Clearer sequencing in get_best_parameters_from_model_predictions_with_trial_index - Removed model fit quality check as part of BPR Previous, redacted changes: - Added argument use_model_only_if_good to force model-based BPR even if model fit is bad Differential Revision: D80019803
hvarfner
force-pushed
the
export-D80019803
branch
from
6528cdd to
334138e
Compare
Contributor
|
This pull request was exported from Phabricator. Differential Revision: D80019803 |
hvarfner
pushed a commit
to hvarfner/Ax
that referenced
this pull request
Aug 19, 2025
…4128) Summary: Pull Request resolved: facebook#4128 This diff addresses two issues in the computation of inference trace: 1. The generation strategy is copied inside run_optimization_with_orchestrator --> we retrieve the traces on an unused generation strategy --> get_best_point defaults to the best raw observation on ALL obserations 2. Relevant data not filtered in the fallback option for get_best_parameters_from_model_predictions_with_trial_index Both of these individually lead to the inference trace being incorrect - the first to the best raw value of ALL trials, the second to the best predicted across ALL trials. Changes: - Moved copying of generation strategy to the level`benchmark_replication`, since results need to be computed on the used `generation_strategy` and not an empty copy. This means that `run_optimization_with_orchestrator` no longer `clone_and_reset`'s the GS. - Clearer sequencing in get_best_parameters_from_model_predictions_with_trial_index - Removed model fit quality check as part of BPR Previous, redacted changes: - Added argument use_model_only_if_good to force model-based BPR even if model fit is bad Differential Revision: D80019803
hvarfner
force-pushed
the
export-D80019803
branch
from
334138e to
63360d3
Compare
hvarfner
pushed a commit
to hvarfner/Ax
that referenced
this pull request
Aug 19, 2025
…4128) Summary: This diff addresses two issues in the computation of inference trace: 1. The generation strategy is copied inside run_optimization_with_orchestrator --> we retrieve the traces on an unused generation strategy --> get_best_point defaults to the best raw observation on ALL obserations 2. Relevant data not filtered in the fallback option for get_best_parameters_from_model_predictions_with_trial_index Both of these individually lead to the inference trace being incorrect - the first to the best raw value of ALL trials, the second to the best predicted across ALL trials. Changes: - Moved copying of generation strategy to the level`benchmark_replication`, since results need to be computed on the used `generation_strategy` and not an empty copy. This means that `run_optimization_with_orchestrator` no longer `clone_and_reset`'s the GS. - Clearer sequencing in get_best_parameters_from_model_predictions_with_trial_index - Removed model fit quality check as part of BPR Previous, redacted changes: - Added argument use_model_only_if_good to force model-based BPR even if model fit is bad Differential Revision: D80019803
hvarfner
pushed a commit
to hvarfner/Ax
that referenced
this pull request
Aug 19, 2025
…4128) Summary: This diff addresses two issues in the computation of inference trace: 1. The generation strategy is copied inside run_optimization_with_orchestrator --> we retrieve the traces on an unused generation strategy --> get_best_point defaults to the best raw observation on ALL obserations 2. Relevant data not filtered in the fallback option for get_best_parameters_from_model_predictions_with_trial_index Both of these individually lead to the inference trace being incorrect - the first to the best raw value of ALL trials, the second to the best predicted across ALL trials. Changes: - Moved copying of generation strategy to the level`benchmark_replication`, since results need to be computed on the used `generation_strategy` and not an empty copy. This means that `run_optimization_with_orchestrator` no longer `clone_and_reset`'s the GS. - Clearer sequencing in get_best_parameters_from_model_predictions_with_trial_index - Removed model fit quality check as part of BPR Previous, redacted changes: - Added argument use_model_only_if_good to force model-based BPR even if model fit is bad Differential Revision: D80019803
hvarfner
force-pushed
the
export-D80019803
branch
from
63360d3 to
8b206e0
Compare
Contributor
|
This pull request was exported from Phabricator. Differential Revision: D80019803 |
hvarfner
pushed a commit
to hvarfner/Ax
that referenced
this pull request
Aug 19, 2025
…4128) Summary: Pull Request resolved: facebook#4128 This diff addresses two issues in the computation of inference trace: 1. The generation strategy is copied inside run_optimization_with_orchestrator --> we retrieve the traces on an unused generation strategy --> get_best_point defaults to the best raw observation on ALL obserations 2. Relevant data not filtered in the fallback option for get_best_parameters_from_model_predictions_with_trial_index Both of these individually lead to the inference trace being incorrect - the first to the best raw value of ALL trials, the second to the best predicted across ALL trials. Changes: - Moved copying of generation strategy to the level`benchmark_replication`, since results need to be computed on the used `generation_strategy` and not an empty copy. This means that `run_optimization_with_orchestrator` no longer `clone_and_reset`'s the GS. - Clearer sequencing in get_best_parameters_from_model_predictions_with_trial_index - Removed model fit quality check as part of BPR Previous, redacted changes: - Added argument use_model_only_if_good to force model-based BPR even if model fit is bad Differential Revision: D80019803
hvarfner
force-pushed
the
export-D80019803
branch
from
8b206e0 to
ef65cae
Compare
Summary: Updates 2026-01-13: Removed cross-validation from best point selection — it was under-tested and redundant. Changes: - best_point.py: Removed assess_model_fit / _is_all_noiseless logic; now always uses model_best_point() directly - test_best_point_utils.py: Removed 2 obsolete tests, fixed 2 test bugs (missing metric_signature, wrong experiment reference) - benchmark.py / test_benchmark.py: Import fixes This diff addresses two issues in the computation of inference trace: 1. The generation strategy is copied inside run_optimization_with_orchestrator --> we retrieve the traces on an unused generation strategy --> get_best_point defaults to the best raw observation on ALL obserations 2. Relevant data not filtered in the fallback option for get_best_parameters_from_model_predictions_with_trial_index Both of these individually lead to the inference trace being incorrect - the first to the best raw value of ALL trials, the second to the best predicted across ALL trials. Changes: - Moved copying of generation strategy to the level`benchmark_replication`, since results need to be computed on the used `generation_strategy` and not an empty copy. This means that `run_optimization_with_orchestrator` no longer `clone_and_reset`'s the GS. - Clearer sequencing in get_best_parameters_from_model_predictions_with_trial_index - Removed model fit quality check as part of BPR Previous, redacted changes: - Added argument use_model_only_if_good to force model-based BPR even if model fit is bad Differential Revision: D80019803
hvarfner
force-pushed
the
export-D80019803
branch
from
ef65cae to
bf99553
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary:
This diff addresses two issues in the computation of inference trace:
The generation strategy is copied inside run_optimization_with_orchestrator --> we retrieve the traces on an unused generation strategy --> get_best_point defaults to the best raw observation on ALL obserations
Relevant data not filtered in the fallback option for get_best_parameters_from_model_predictions_with_trial_index
Both of these individually lead to the inference trace being incorrect - the first to the best raw value of ALL trials, the second to the best predicted across ALL trials.
Changes:
Differential Revision: D80019803