Make TransformToNewSQ handle non-relativizable trials (#5039)#5039
Closed
ItsMrLin wants to merge 3 commits intofacebook:mainfrom
Closed
Make TransformToNewSQ handle non-relativizable trials (#5039)#5039ItsMrLin wants to merge 3 commits intofacebook:mainfrom
ItsMrLin wants to merge 3 commits intofacebook:mainfrom
Conversation
meta-cla
bot
added
the
CLA Signed
meta-codesync
bot
changed the title
ItsMrLin
force-pushed
the
export-D96574758
branch
from
52ae8ae to
41b63da
Compare
ItsMrLin
added a commit
to ItsMrLin/Ax
that referenced
this pull request
Mar 17, 2026
Summary: When LILO labeling trials (carrying only pairwise preference data) coexist with Sobol/BO trials (carrying base metrics), TransformToNewSQ and BaseRelativize must handle two distinct concerns: 1. **LILO trials must not be transformed (trial-level).** LILO labeling trials carry only pairwise preference labels — they have no base metrics, so there is no status quo data to relativize against. These trials are excluded from transformation entirely via `_lilo_trial_indices` (built at init from `trial_type == LILO_LABELING`). Their SQ arms are also preserved (not dropped) because PairwiseGP requires both arms in each pairwise comparison. 2. **The preference metric may be missing from non-LILO trials (metric-level).** After `configure_lilo()`, the optimization config includes `pairwise_preference_query`. But Sobol/BO trials were created before LILO was configured — they have data for base metrics like `branin` but not for the preference metric. This is expected. Without special handling, the transform would error or skip the entire trial, blocking relativization of base metrics that *do* have valid data. Per-metric masking solves this: for each metric, only trials whose SQ data includes that metric are relativized; others are left untouched via NaN placeholders. In short: | Concern | What | How | | ----------------------|--------------------------------|----------------------------| | LILO trials | Skip entire rows by trial idx | `_lilo_trial_indices` mask| | Preference metric | Skip per-metric when SQ absent | NaN placeholders + mask| The first is a hard exclusion (these trials should never be touched). The second is a graceful degradation (this metric is absent from some trials, but other metrics in those same trials should still be relativized normally). Differential Revision: D96574758
ItsMrLin
added a commit
to ItsMrLin/Ax
that referenced
this pull request
Mar 17, 2026
Summary: Pull Request resolved: facebook#5039 When LILO labeling trials (carrying only pairwise preference data) coexist with Sobol/BO trials (carrying base metrics), TransformToNewSQ and BaseRelativize must handle two distinct concerns: 1. **LILO trials must not be transformed (trial-level).** LILO labeling trials carry only pairwise preference labels — they have no base metrics, so there is no status quo data to relativize against. These trials are excluded from transformation entirely via `_lilo_trial_indices` (built at init from `trial_type == LILO_LABELING`). Their SQ arms are also preserved (not dropped) because PairwiseGP requires both arms in each pairwise comparison. 2. **The preference metric may be missing from non-LILO trials (metric-level).** After `configure_lilo()`, the optimization config includes `pairwise_preference_query`. But Sobol/BO trials were created before LILO was configured — they have data for base metrics like `branin` but not for the preference metric. This is expected. Without special handling, the transform would error or skip the entire trial, blocking relativization of base metrics that *do* have valid data. Per-metric masking solves this: for each metric, only trials whose SQ data includes that metric are relativized; others are left untouched via NaN placeholders. In short: | Concern | What | How | | ----------------------|--------------------------------|----------------------------| | LILO trials | Skip entire rows by trial idx | `_lilo_trial_indices` mask| | Preference metric | Skip per-metric when SQ absent | NaN placeholders + mask| The first is a hard exclusion (these trials should never be touched). The second is a graceful degradation (this metric is absent from some trials, but other metrics in those same trials should still be relativized normally). Differential Revision: D96574758
ItsMrLin
force-pushed
the
export-D96574758
branch
from
41b63da to
6d40ddc
Compare
ItsMrLin
added a commit
to ItsMrLin/Ax
that referenced
this pull request
Mar 18, 2026
Summary: When LILO labeling trials (carrying only pairwise preference data) coexist with Sobol/BO trials (carrying base metrics), TransformToNewSQ and BaseRelativize must handle two distinct concerns: 1. **LILO trials must not be transformed (trial-level).** LILO labeling trials carry only pairwise preference labels — they have no base metrics, so there is no status quo data to relativize against. These trials are excluded from transformation entirely via `_lilo_trial_indices` (built at init from `trial_type == LILO_LABELING`). Their SQ arms are also preserved (not dropped) because PairwiseGP requires both arms in each pairwise comparison. 2. **The preference metric may be missing from non-LILO trials (metric-level).** After `configure_lilo()`, the optimization config includes `pairwise_preference_query`. But Sobol/BO trials were created before LILO was configured — they have data for base metrics like `branin` but not for the preference metric. This is expected. Without special handling, the transform would error or skip the entire trial, blocking relativization of base metrics that *do* have valid data. Per-metric masking solves this: for each metric, only trials whose SQ data includes that metric are relativized; others are left untouched via NaN placeholders. In short: | Concern | What | How | | ----------------------|--------------------------------|----------------------------| | LILO trials | Skip entire rows by trial idx | `_lilo_trial_indices` mask| | Preference metric | Skip per-metric when SQ absent | NaN placeholders + mask| The first is a hard exclusion (these trials should never be touched). The second is a graceful degradation (this metric is absent from some trials, but other metrics in those same trials should still be relativized normally). Reviewed By: Balandat Differential Revision: D96574758
ItsMrLin
force-pushed
the
export-D96574758
branch
from
6d40ddc to
d6fa755
Compare
ItsMrLin
added a commit
to ItsMrLin/Ax
that referenced
this pull request
Mar 18, 2026
Summary: Pull Request resolved: facebook#5039 When LILO labeling trials (carrying only pairwise preference data) coexist with Sobol/BO trials (carrying base metrics), TransformToNewSQ and BaseRelativize must handle two distinct concerns: 1. **LILO trials must not be transformed (trial-level).** LILO labeling trials carry only pairwise preference labels — they have no base metrics, so there is no status quo data to relativize against. These trials are excluded from transformation entirely via `_lilo_trial_indices` (built at init from `trial_type == LILO_LABELING`). Their SQ arms are also preserved (not dropped) because PairwiseGP requires both arms in each pairwise comparison. 2. **The preference metric may be missing from non-LILO trials (metric-level).** After `configure_lilo()`, the optimization config includes `pairwise_preference_query`. But Sobol/BO trials were created before LILO was configured — they have data for base metrics like `branin` but not for the preference metric. This is expected. Without special handling, the transform would error or skip the entire trial, blocking relativization of base metrics that *do* have valid data. Per-metric masking solves this: for each metric, only trials whose SQ data includes that metric are relativized; others are left untouched via NaN placeholders. In short: | Concern | What | How | | ----------------------|--------------------------------|----------------------------| | LILO trials | Skip entire rows by trial idx | `_lilo_trial_indices` mask| | Preference metric | Skip per-metric when SQ absent | NaN placeholders + mask| The first is a hard exclusion (these trials should never be touched). The second is a graceful degradation (this metric is absent from some trials, but other metrics in those same trials should still be relativized normally). Reviewed By: Balandat Differential Revision: D96574758
ItsMrLin
force-pushed
the
export-D96574758
branch
from
d6fa755 to
abf5136
Compare
meta-codesync
bot
changed the title
ItsMrLin
added a commit
to ItsMrLin/Ax
that referenced
this pull request
Mar 18, 2026
Summary: When LILO labeling trials (carrying only pairwise preference data) coexist with Sobol/BO trials (carrying base metrics), TransformToNewSQ and BaseRelativize must skip trials that cannot be relativized. `TorchAdapter._transform_data` (D96574732) pops the pairwise preference column before any Ax transforms run, so TransformToNewSQ only ever sees base metrics. This means the only concern at this layer is trial-level exclusion: LILO labeling trials have no base metrics and no status quo data to relativize against. Changes: - **`BaseRelativize.__init__`**: Collects `_non_relativizable_trial_indices` from trials with `trial_type == LILO_LABELING`. - **`BaseRelativize.transform_experiment_data`**: Excludes non-relativizable trials from the missing-SQ validation check and from relativization. - **`TransformToNewSQ.transform_experiment_data`**: Skips target trial, non-relativizable trials, and trials without SQ data. Drops SQ arms only from actually-transformed trials (non-transformed trials keep all arms, which PairwiseGP requires). - **`TransformToNewSQ._get_relative_data_from_obs`**: Returns data unchanged for trials without SQ data. Reviewed By: Balandat Differential Revision: D96574758
ItsMrLin
force-pushed
the
export-D96574758
branch
from
abf5136 to
2403f0d
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #5039 +/- ##
==========================================
- Coverage 96.73% 96.73% -0.01%
==========================================
Files 606 606
Lines 66242 66370 +128
==========================================
+ Hits 64080 64203 +123
- Misses 2162 2167 +5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Summary: Move `LLMMessage` dict conversion from the `experiment.llm_messages` getter/setter to the storage encoders/decoders, following Ax convention that domain objects hold domain types and serialization happens at the storage boundary. **`experiment.py`**: The setter now stores `LLMMessage` objects directly in `_properties`. The getter handles both `LLMMessage` objects (new path) and plain dicts (backward compat with previously stored data). **JSON store**: No explicit changes needed — the encoder's generic dataclass fallback auto-serializes `LLMMessage` with a `__type` tag, and `LLMMessage` is already registered in `CORE_DECODER_REGISTRY`. **SQA store**: The encoder converts `LLMMessage` → dict via `dataclasses.asdict()` in the properties copy before DB write (same pattern as `pruning_target_parameterization`). The decoder converts dicts → `LLMMessage` after loading properties, in both `_init_experiment_from_sqa` and `_init_mt_experiment_from_sqa`. Differential Revision: D96434290
Summary: In LILO (LLM-In-the-Loop Optimization) experiments, the optimization config objective is `pairwise_pref_query` — a derived metric that only LILO labeling trials carry data for. `get_target_trial_index()` then selects these labeling trials (which have COMPLETE pairwise data) as the relativization reference instead of non-LILO trials (which have base metric data). The target trial's SQ then lacks base metrics, causing TransformToNewSQ and downstream model fitting to fail. Fix: 1. Exclude LILO labeling trials (`trial_type == LILO_LABELING`) from the target trial candidate set. 2. For LILO experiments, accept INCOMPLETE metric availability so that non-LILO trials (which have base-metric data but lack the pairwise preference metric) can serve as relativization references. Differential Revision: D96574746
ItsMrLin
force-pushed
the
export-D96574758
branch
from
2403f0d to
2187978
Compare
ItsMrLin
added a commit
to ItsMrLin/Ax
that referenced
this pull request
Mar 18, 2026
Summary: When LILO labeling trials (carrying only pairwise preference data) coexist with Sobol/BO trials (carrying base metrics), TransformToNewSQ and BaseRelativize must skip trials that cannot be relativized. `TorchAdapter._transform_data` (D96574732) pops the pairwise preference column before any Ax transforms run, so TransformToNewSQ only ever sees base metrics. This means the only concern at this layer is trial-level exclusion: LILO labeling trials have no base metrics and no status quo data to relativize against. Changes: - **`BaseRelativize.__init__`**: Collects `_non_relativizable_trial_indices` from trials with `trial_type == LILO_LABELING`. - **`BaseRelativize.transform_experiment_data`**: Excludes non-relativizable trials from the missing-SQ validation check and from relativization. - **`TransformToNewSQ.transform_experiment_data`**: Skips target trial, non-relativizable trials, and trials without SQ data. Drops SQ arms only from actually-transformed trials (non-transformed trials keep all arms, which PairwiseGP requires). - **`TransformToNewSQ._get_relative_data_from_obs`**: Returns data unchanged for trials without SQ data. Reviewed By: Balandat Differential Revision: D96574758
ItsMrLin
force-pushed
the
export-D96574758
branch
from
2187978 to
f3a42f0
Compare
ItsMrLin
added a commit
to ItsMrLin/Ax
that referenced
this pull request
Mar 18, 2026
Summary: When LILO labeling trials (carrying only pairwise preference data) coexist with Sobol/BO trials (carrying base metrics), TransformToNewSQ and BaseRelativize must skip trials that cannot be relativized. `TorchAdapter._transform_data` (D96574732) pops the pairwise preference column before any Ax transforms run, so TransformToNewSQ only ever sees base metrics. This means the only concern at this layer is trial-level exclusion: LILO labeling trials have no base metrics and no status quo data to relativize against. Changes: - **`BaseRelativize.__init__`**: Collects `_non_relativizable_trial_indices` from trials with `trial_type == LILO_LABELING`. - **`BaseRelativize.transform_experiment_data`**: Excludes non-relativizable trials from the missing-SQ validation check and from relativization. - **`TransformToNewSQ.transform_experiment_data`**: Skips target trial, non-relativizable trials, and trials without SQ data. Drops SQ arms only from actually-transformed trials (non-transformed trials keep all arms, which PairwiseGP requires). - **`TransformToNewSQ._get_relative_data_from_obs`**: Returns data unchanged for trials without SQ data. Reviewed By: Balandat Differential Revision: D96574758
Summary: Pull Request resolved: facebook#5039 When LILO labeling trials (carrying only pairwise preference data) coexist with Sobol/BO trials (carrying base metrics), TransformToNewSQ and BaseRelativize must skip trials that cannot be relativized. `TorchAdapter._transform_data` (D96574732) pops the pairwise preference column before any Ax transforms run, so TransformToNewSQ only ever sees base metrics. This means the only concern at this layer is trial-level exclusion: LILO labeling trials have no base metrics and no status quo data to relativize against. Changes: - **`BaseRelativize.__init__`**: Collects `_non_relativizable_trial_indices` from trials with `trial_type == LILO_LABELING`. - **`BaseRelativize.transform_experiment_data`**: Excludes non-relativizable trials from the missing-SQ validation check and from relativization. - **`TransformToNewSQ.transform_experiment_data`**: Skips target trial, non-relativizable trials, and trials without SQ data. Drops SQ arms only from actually-transformed trials (non-transformed trials keep all arms, which PairwiseGP requires). - **`TransformToNewSQ._get_relative_data_from_obs`**: Returns data unchanged for trials without SQ data. Reviewed By: Balandat Differential Revision: D96574758
ItsMrLin
force-pushed
the
export-D96574758
branch
from
f3a42f0 to
531d8af
Compare
|
This pull request has been merged in 84838db. |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 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:
When LILO labeling trials (carrying only pairwise preference data)
coexist with Sobol/BO trials (carrying base metrics), TransformToNewSQ
and BaseRelativize must skip trials that cannot be relativized.
TorchAdapter._transform_data(D96574732) pops the pairwise preferencecolumn before any Ax transforms run, so TransformToNewSQ only ever sees
base metrics. This means the only concern at this layer is trial-level
exclusion: LILO labeling trials have no base metrics and no status quo
data to relativize against.
Changes:
BaseRelativize.__init__: Collects_non_relativizable_trial_indicesfrom trials with
trial_type == LILO_LABELING.BaseRelativize.transform_experiment_data: Excludes non-relativizabletrials from the missing-SQ validation check and from relativization.
TransformToNewSQ.transform_experiment_data: Skips target trial,non-relativizable trials, and trials without SQ data. Drops SQ arms
only from actually-transformed trials (non-transformed trials keep
all arms, which PairwiseGP requires).
TransformToNewSQ._get_relative_data_from_obs: Returns dataunchanged for trials without SQ data.
Reviewed By: Balandat
Differential Revision: D96574758