Fixing incompatible device in client.compute_analyses (no ruff) #3839
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
facebook-github-bot
added
the
CLA Signed
1 task
Balandat
requested changes
May 30, 2025
Contributor
Balandat
left a comment
Balandat
left a comment
There was a problem hiding this comment.
Thanks, lgtm apart from a couple of nits.
Usually we'd want to update the unit test to cover this behavior but it may make more sense to move some of the non-Ax-related sensitivity code to botorch and use the test infra for GPU testing there.
Balandat
approved these changes
Jun 1, 2025
Contributor
|
@Balandat has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
Contributor
Author
|
Alright, it looks like passing Not sure why this happens, but I am pushing a fix now. |
Contributor
|
@Balandat has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3839 +/- ##
=======================================
Coverage 95.86% 95.87%
=======================================
Files 553 554 +1
Lines 54612 54663 +51
=======================================
+ Hits 52356 52410 +54
+ Misses 2256 2253 -3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Contributor
Author
|
@Balandat, do I need to do anything else to have this pushed into the main or just wait? |
Contributor
|
@VMLC-PV nope all good we'll be merging this in very soon! |
Contributor
mpolson64
added a commit
that referenced
this pull request
Aug 12, 2025
* New ExperimentData container (for modeling layer) (#3851)
Summary:
Pull Request resolved: https://github.com/facebook/Ax/pull/3851
Introduces `ExperimentData`, a container of two dataframes that is intended to replace the use of lists of `Observation` objects within the `Adapter` and `Transform` layers. By replacing lists of objects with dataframes, we enable vectorized operations (replacing many nested for loops), which results in 4-5x speed up in data processing within Adapter.
arm_data: A dataframe containing columns for trial index, arm name, and
the parameterization of each arm. Each row corresponds to
a (trial_index, arm_name) pair.
observation_data: A dataframe containing columns for trial index, arm name,
any map keys (e.g., progression / step), and the mean and sem observations
for the metrics. Each row corresponds to an observation for the given
trial index, arm name, and map key values / progression.
This is typically constructed by pivoting `(Map)Data.true_df`.
Reviewed By: esantorella
Differential Revision: D69943574
fbshipit-source-id: 26a9f77a2d8ef58b7bffa3ac37ffb5e317bd9b88
* Eliminate duplicate underscore in OneHot encoded parameter names (#3858)
Summary:
Pull Request resolved: https://github.com/facebook/Ax/pull/3858
`OneHot` transform had a duplicate `_` in the transformed parameter names. This diff removes that. With this change, the transformed parameter name will be
- For two values only: `parameter_OH_PARAM`, which was `parameter_OH_PARAM_`
- For more values: `parameter_OH_PARAM_i` which was `parameter_OH_PARAM__i`.
Reviewed By: Balandat
Differential Revision: D75892920
fbshipit-source-id: f5980968dca45ab7403af913b371c5a35334aa29
* Use existing data type instead of default one when joining Data objects in Experiment (#3859)
Summary:
Pull Request resolved: https://github.com/facebook/Ax/pull/3859
Allow the `Data` objects in an `Experiment` to be of a different type than `experiment.default_data_constructor()`
Reviewed By: esantorella
Differential Revision: D75890784
fbshipit-source-id: c1c6eeaccea750f8cddd27a3925ad75e9d485c43
* Refactor auxiliary experiment storage (again) (#3829)
Summary: Pull Request resolved: https://github.com/facebook/Ax/pull/3829
Reviewed By: lena-kashtelyan
Differential Revision: D74902347
fbshipit-source-id: 9876ab3d27c040130ca4b3b0acf85a2e6b531747
* Add metadata to ExperimentData (#3852)
Summary:
Pull Request resolved: https://github.com/facebook/Ax/pull/3852
This diff adds a `metadata` column to `arm_data` of `ExperimentData`, which was introduced in the previous diff. The `metadata` is extracted from the `GeneratorRun.candidate_metadata_by_arm_signature` (thus available at `(trial_index, arm_name)` level, same as the index of `arm_data`), and is traditionally present as `ObservationFeatures.metadata`. This is extracted in `TorchAdapter._fit` and passed down to `Generator.fit` (only to be used by TRBO AFAICT).
Alternative: We could add metadata as a separate dataframe on `ExperimentData` but I don't see a clear benefit from doing so.
- Pros: It'd eliminate the need to exclude `metadata` column when accessing only the parameterizations from `arm_data`, which we need to do in a few places.
- Cons: When extracting the metadata in `Adapter.fit`, we would have additional indexing operations to ensure that the `metadata` matches the index of `arm_data` and `observation_data`.
Reviewed By: esantorella
Differential Revision: D72801799
fbshipit-source-id: 22339f159e3469bee399a8760beb5ff1941e5307
* CenterGenerationNode: Fallback to Sobol if center is infeasible (#3863)
Summary:
Pull Request resolved: https://github.com/facebook/Ax/pull/3863
As titled. Checks for feasibility of the generated candidate and falls back to Sobol if it is infeasible.
Fixes https://github.com/facebook/Ax/issues/3862
Reviewed By: Balandat
Differential Revision: D75956688
fbshipit-source-id: bea28b755b16d7a5c06a10d1334de993c635fe58
* Fixing incompatible device in client.compute_analyses (#3839)
Summary:
Making sure that the torch_device is set to the same as the model in the client.compute_analyses in case CUDA is used in the Modular BoTorch Interface.
Related to the issue [https://github.com/facebook/Ax/issues/3813](https://github.com/facebook/Ax/issues/3813)
and without the messed-up formatting from https://github.com/facebook/Ax/issues/3833, following Balandat's advice.
Pull Request resolved: https://github.com/facebook/Ax/pull/3839
Reviewed By: hvarfner
Differential Revision: D75758870
Pulled By: Balandat
fbshipit-source-id: bec3a7c51219a334192db390f1ed93d4d9571929
* Update Trial Status Filtering in Plots (#3854)
Summary:
Pull Request resolved: https://github.com/facebook/Ax/pull/3854
Update `ArmEffectsPlot` and `ScatterPlot` to accept all `TrialStatus`'s that are expecting data, not just `COMPLETED`.
Reviewed By: sdaulton
Differential Revision: D75886336
fbshipit-source-id: 9a55cb7b1ccde48783e387be4e8c9da8b56a9c08
* Color constant addition (#3869)
Summary:
Pull Request resolved: https://github.com/facebook/Ax/pull/3869
This commit adds a new color_constants.py file to that centralizes color definitions for Ax visualizations. The file imports base colors creates semantic aliases that clearly describe their usage contexts.
The primary goal is to encourage consistent color usage across the Ax visualization system by:
1. Providing descriptive, semantic names for colors based on their functional purpose
2. Creating a single source of truth for color definitions
3. Making color schemes more discoverable for developers
Reviewed By: eonofrey
Differential Revision: D75886310
fbshipit-source-id: 719708ddc163d96c527aa50385befa96dfe91714
* Adding auxiliary experiment methods to Experiment class (#3861)
Summary:
Pull Request resolved: https://github.com/facebook/Ax/pull/3861
Adding auxiliary experiment methods to Experiment class to add (as first experiment) and find auxiliary experiment.
Meta: Add to first is needed as we currently support one aux PE experiment and the first one is the one with highest priority and is the one that will be used. Find aux experiment is needed to support pref opt config (see the diff on top), which will allow us to identify which PE exp to use instead of relying on the order.
Reviewed By: mgarrard
Differential Revision: D75696082
fbshipit-source-id: cff96ee75ba29f5831e3136ee47df0d7d931f9b3
* Typo in `FAILURE_EXCEEDED_MSG` (#3870)
Summary:
Pull Request resolved: https://github.com/facebook/Ax/pull/3870
Fixing a typo in an error message.
Reviewed By: bernardbeckerman
Differential Revision: D75987184
fbshipit-source-id: bd27b2766889442e0a9e3cf31c2464e61dcdc691
* Rename gen to gen_single_trial (#3844)
Summary:
Pull Request resolved: https://github.com/facebook/Ax/pull/3844
This diff updates the current `gen` method to be called `gen_single_trial` -- we will update gen_multiple_trials_with_multiple_models to be called gen in the next diff (initially tried to do them in the same diff and it was very confusing).
I decided to replace everything that currently calls gen with gen_single_trial instead of replace some of them with gen_multi as i think it kept things cleaner. This primarily effects test files.
Reviewed By: lena-kashtelyan, esantorella
Differential Revision: D75688420
fbshipit-source-id: 2b338f56fcd56e237394b52a609d92e1e2293b7a
* rename gen_for_multiple_trials_with_mulitple_nodes to gen (#3846)
Summary:
Pull Request resolved: https://github.com/facebook/Ax/pull/3846
We want the most powerful gen to be our default gen, and that's gen_for_multi_with_multi, in the previous diff we free'd up the name gen
Reviewed By: saitcakmak
Differential Revision: D75718755
fbshipit-source-id: 4670e5f68d7828ca32d965ee614f01ad274c3726
* Add ExperimentData input to Transform constructors (#3856)
Summary:
Pull Request resolved: https://github.com/facebook/Ax/pull/3856
We will switch to using `ExperimentData` rather than `list[Observation]` within the `Adapter`, which necessitates replacing `list[Observation]` input to `Transform` constructors with `ExperimentData`, and supporting transforms with `ExperimentData`.
This diff only updates the `Transform` constructors to accept the new argument and adds `Transform.transform_experiment_data` to the base class. The transforms will be updated in following diffs to make use of the new argument in the constructor and implement the `transform_experiment_data` method.
Background: As part of the larger refactor, we will be using `ExperimentData` in place of `list[Observation]` within the `Adapter`.
- The transforms will be initialized using `ExperimentData`. The `observations` input to the constructors may be deprecated once the use cases are updated.
- The training data for `Adapter` will be represented with `ExperimentData` and will be transformed using `transform_experiment_data`.
- For misc input / output to various `Adapter` and other methods, the `Observation / ObservationFeatures / ObservationData` objects will remain. To support these, we will retain the existing transform methods that service these objects.
- Since `ExperimentData` is not planned to be used as an output of user facing methods, we do not need to untransform it. We are not planning to implement`untransform_experiment_data`.
Reviewed By: esantorella
Differential Revision: D72664137
fbshipit-source-id: 27843bcaa751676f5684a7d0b8c52e4cbe46a75e
* Implement IntToFloat.transform_experiment_data (#3871)
Summary:
Pull Request resolved: https://github.com/facebook/Ax/pull/3871
As titled. Supports transforming `ExperimentData` with `IntToFloat` transform.
Background: As part of the larger refactor, we will be using `ExperimentData` in place of `list[Observation]` within the `Adapter`.
- The transforms will be initialized using `ExperimentData`. The `observations` input to the constructors may be deprecated once the use cases are updated.
- The training data for `Adapter` will be represented with `ExperimentData` and will be transformed using `transform_experiment_data`.
- For misc input / output to various `Adapter` and other methods, the `Observation / ObservationFeatures / ObservationData` objects will remain. To support these, we will retain the existing transform methods that service these objects.
- Since `ExperimentData` is not planned to be used as an output of user facing methods, we do not need to untransform it. We are not planning to implement`untransform_experiment_data`.
Reviewed By: esantorella
Differential Revision: D75903694
fbshipit-source-id: 8cad41c0734a7456ff4539a3b824df9100f8f578
* Implement Log.transform_experiment_data (#3872)
Summary:
Pull Request resolved: https://github.com/facebook/Ax/pull/3872
As titled. Supports transforming `ExperimentData` with `Log` transform.
Background: As part of the larger refactor, we will be using `ExperimentData` in place of `list[Observation]` within the `Adapter`.
- The transforms will be initialized using `ExperimentData`. The `observations` input to the constructors may be deprecated once the use cases are updated.
- The training data for `Adapter` will be represented with `ExperimentData` and will be transformed using `transform_experiment_data`.
- For misc input / output to various `Adapter` and other methods, the `Observation / ObservationFeatures / ObservationData` objects will remain. To support these, we will retain the existing transform methods that service these objects.
- Since `ExperimentData` is not planned to be used as an output of user facing methods, we do not need to untransform it. We are not planning to implement`untransform_experiment_data`.
Reviewed By: SebastianAment
Differential Revision: D75905207
fbshipit-source-id: 5950f64a6b2757f8fd5173433585ea198d4fc232
* Changes throughtout to reflect addition of color constants (#3877)
Summary:
Pull Request resolved: https://github.com/facebook/Ax/pull/3877
Updates naming of color constants used throughout the Ax codebase
Reviewed By: eonofrey
Differential Revision: D76043167
fbshipit-source-id: aaed8c346aeb2703c8b66f5355a91b520f9e7078
* Deprecate `no_bayesian_optimization` (#3857)
Summary:
Pull Request resolved: https://github.com/facebook/Ax/pull/3857
The `no_bayesian_optimization` arg has been replaced by `force_random_search`, and deprecation messages have been in place for 9 months (https://github.com/facebook/Ax/pull/2693). This diff deprecates.
Reviewed By: ltiao, SebastianAment, saitcakmak
Differential Revision: D75891247
fbshipit-source-id: e75464390ba24e8f0d85afe423af1c9df944fa30
* Addressing strange status quo dotted line on Scatter (#3884)
Summary:
Pull Request resolved: https://github.com/facebook/Ax/pull/3884
included is a minor fix that checks status of status_quo and doesn't draw the line if it is a candidate. It does that by checking if the metric mean is calculated or not.
Here is a working demo: https://fburl.com/anp/84ecgfep
Reviewed By: mpolson64
Differential Revision: D75976043
fbshipit-source-id: ad75c05a6dbdbeb5b20e673ab3b67026798b74fb
* Implement RemoveFixed.transform_experiment_data (#3873)
Summary:
Pull Request resolved: https://github.com/facebook/Ax/pull/3873
As titled. Supports transforming `ExperimentData` with `RemoveFixed` transform.
Background: As part of the larger refactor, we will be using `ExperimentData` in place of `list[Observation]` within the `Adapter`.
- The transforms will be initialized using `ExperimentData`. The `observations` input to the constructors may be deprecated once the use cases are updated.
- The training data for `Adapter` will be represented with `ExperimentData` and will be transformed using `transform_experiment_data`.
- For misc input / output to various `Adapter` and other methods, the `Observation / ObservationFeatures / ObservationData` objects will remain. To support these, we will retain the existing transform methods that service these objects.
- Since `ExperimentData` is not planned to be used as an output of user facing methods, we do not need to untransform it. We are not planning to implement`untransform_experiment_data`.
Reviewed By: esantorella
Differential Revision: D75906004
fbshipit-source-id: 39403b3c791bc7514a4e6771a3d773e41006857a
* Implement ChoiceToNumericChoice.transform_experiment_data (#3874)
Summary:
Pull Request resolved: https://github.com/facebook/Ax/pull/3874
As titled. Supports transforming `ExperimentData` with `ChoiceToNumericChoice` transform and subclasses.
Background: As part of the larger refactor, we will be using `ExperimentData` in place of `list[Observation]` within the `Adapter`.
- The transforms will be initialized using `ExperimentData`. The `observations` input to the constructors may be deprecated once the use cases are updated.
- The training data for `Adapter` will be represented with `ExperimentData` and will be transformed using `transform_experiment_data`.
- For misc input / output to various `Adapter` and other methods, the `Observation / ObservationFeatures / ObservationData` objects will remain. To support these, we will retain the existing transform methods that service these objects.
- Since `ExperimentData` is not planned to be used as an output of user facing methods, we do not need to untransform it. We are not planning to implement`untransform_experiment_data`.
Reviewed By: esantorella
Differential Revision: D75967458
fbshipit-source-id: 9f70c35f85b80761df2a87adbf1db433cbb3fe7b
* Implement FillMissingParameters.transform_experiment_data (#3875)
Summary:
Pull Request resolved: https://github.com/facebook/Ax/pull/3875
As titled. Supports transforming `ExperimentData` with `FillMissingParameters` transform.
Background: As part of the larger refactor, we will be using `ExperimentData` in place of `list[Observation]` within the `Adapter`.
- The transforms will be initialized using `ExperimentData`. The `observations` input to the constructors may be deprecated once the use cases are updated.
- The training data for `Adapter` will be represented with `ExperimentData` and will be transformed using `transform_experiment_data`.
- For misc input / output to various `Adapter` and other methods, the `Observation / ObservationFeatures / ObservationData` objects will remain. To support these, we will retain the existing transform methods that service these objects.
- Since `ExperimentData` is not planned to be used as an output of user facing methods, we do not need to untransform it. We are not planning to implement`untransform_experiment_data`.
Reviewed By: esantorella
Differential Revision: D75972103
fbshipit-source-id: 739f332fc2e8d7ae1f749ff7e3562ba0e1a9ce7a
* Implement Logit.transform_experiment_data (#3876)
Summary:
Pull Request resolved: https://github.com/facebook/Ax/pull/3876
As titled. Supports transforming `ExperimentData` with `Logit` transform.
Background: As part of the larger refactor, we will be using `ExperimentData` in place of `list[Observation]` within the `Adapter`.
- The transforms will be initialized using `ExperimentData`. The `observations` input to the constructors may be deprecated once the use cases are updated.
- The training data for `Adapter` will be represented with `ExperimentData` and will be transformed using `transform_experiment_data`.
- For misc input / output to various `Adapter` and other methods, the `Observation / ObservationFeatures / ObservationData` objects will remain. To support these, we will retain the existing transform methods that service these objects.
- Since `ExperimentData` is not planned to be used as an output of user facing methods, we do not need to untransform it. We are not planning to implement`untransform_experiment_data`.
Reviewed By: esantorella
Differential Revision: D75973054
fbshipit-source-id: a23f9c27f396e5328f0f83293ec9454c894a7788
* Implement OneHot.transform_experiment_data (#3886)
Summary:
Pull Request resolved: https://github.com/facebook/Ax/pull/3886
As titled. Supports transforming `ExperimentData` with `OneHot` transform.
Background: As part of the larger refactor, we will be using `ExperimentData` in place of `list[Observation]` within the `Adapter`.
- The transforms will be initialized using `ExperimentData`. The `observations` input to the constructors may be deprecated once the use cases are updated.
- The training data for `Adapter` will be represented with `ExperimentData` and will be transformed using `transform_experiment_data`.
- For misc input / output to various `Adapter` and other methods, the `Observation / ObservationFeatures / ObservationData` objects will remain. To support these, we will retain the existing transform methods that service these objects.
- Since `ExperimentData` is not planned to be used as an output of user facing methods, we do not need to untransform it. We are not planning to implement`untransform_experiment_data`.
Reviewed By: esantorella
Differential Revision: D76074577
fbshipit-source-id: 07f17b5f91a7e1ce5c6d9eb91dd034bec17d6f89
* Speed up sensitivity analyses by using batch dimension (#3891)
Summary:
Pull Request resolved: https://github.com/facebook/Ax/pull/3891
* Speed up sensitivity computations by using batch dimension -- use an x of shape [n, 1, d] instead of [n, d]. This avoids the costly creation of an (n, n) covariance matrix when we only need its diagonal elements.
* Stop doing those computations in minibatches, which was done to avoid the superlinear memory usage from large batch sizes (Use mini batches in SobolSensitivityGPMean #1848 ), which came from the (n,n) covariance matrix we no longer compute.
Reviewed By: Balandat
Differential Revision: D75712208
fbshipit-source-id: 19714227a6f0124064b3f4576ab8bd34eca3c708
* Remove unused arguments from `SobolSensitivityGPMean` (#3892)
Summary:
Pull Request resolved: https://github.com/facebook/Ax/pull/3892
Two arguments are no longer used, one as a result of D75712208 and one has long been unused
Reviewed By: saitcakmak
Differential Revision: D76151380
fbshipit-source-id: f9572bb76282b90001c1c50bbe173900bc8e4b11
* Implement BilogY.transform_experiment_data (#3887)
Summary:
Pull Request resolved: https://github.com/facebook/Ax/pull/3887
As titled. Supports transforming `ExperimentData` with `BilogY` transform. Also removes unnecessary `DataRequiredError`, since the transform does not actually utilize the provided data in the constructor.
Background: As part of the larger refactor, we will be using `ExperimentData` in place of `list[Observation]` within the `Adapter`.
- The transforms will be initialized using `ExperimentData`. The `observations` input to the constructors may be deprecated once the use cases are updated.
- The training data for `Adapter` will be represented with `ExperimentData` and will be transformed using `transform_experiment_data`.
- For misc input / output to various `Adapter` and other methods, the `Observation / ObservationFeatures / ObservationData` objects will remain. To support these, we will retain the existing transform methods that service these objects.
- Since `ExperimentData` is not planned to be used as an output of user facing methods, we do not need to untransform it. We are not planning to implement`untransform_experiment_data`.
Reviewed By: esantorella
Differential Revision: D76081492
fbshipit-source-id: e4c59f64ffc9a9e34701be9db54ad2b9e81569ef
* Implement UnitX.transform_experiment_data (#3888)
Summary:
Pull Request resolved: https://github.com/facebook/Ax/pull/3888
As titled. Supports transforming `ExperimentData` with `UnitX` transform.
Background: As part of the larger refactor, we will be using `ExperimentData` in place of `list[Observation]` within the `Adapter`.
- The transforms will be initialized using `ExperimentData`. The `observations` input to the constructors may be deprecated once the use cases are updated.
- The training data for `Adapter` will be represented with `ExperimentData` and will be transformed using `transform_experiment_data`.
- For misc input / output to various `Adapter` and other methods, the `Observation / ObservationFeatures / ObservationData` objects will remain. To support these, we will retain the existing transform methods that service these objects.
- Since `ExperimentData` is not planned to be used as an output of user facing methods, we do not need to untransform it. We are not planning to implement`untransform_experiment_data`.
Reviewed By: esantorella
Differential Revision: D76085733
fbshipit-source-id: 40a07675cfc4252f9d5699423e6e2bfea5295378
* Re-enable slow sensitivity plot tests (#3893)
Summary:
Pull Request resolved: https://github.com/facebook/Ax/pull/3893
These tests were removed in D74846187 because they were slow. D75569748 and D75712208 sped them up.
Reviewed By: saitcakmak
Differential Revision: D75712258
fbshipit-source-id: 24dd33697d9db4ec6bcc9ebf4a727181a8873906
* Sort experiment data by trial_index, arm_name, and metrics (#3885)
Summary:
Pull Request resolved: https://github.com/facebook/Ax/pull/3885
Arm order in metric results were displayed per the data order. This led to a random ordering of arms in the UI {F1977891605}
To fix this, we want to sort the arm ordering in data.
Data will be sorted by trail_index, then arm_name. Arm name will be sorted as 'custom_name' < '0_1' < '0_2' < '0_11' < '0_100'
Reviewed By: lena-kashtelyan, saitcakmak
Differential Revision: D74409276
fbshipit-source-id: 51d4de4e50f20ac009ed29ec58d4a417be2857a1
* Always create new docusaurus version, only keep latest patch fo… (#3898)
Summary:
…r given major.minor version
Before this change we would only create new versions in docusaurus if the major or minor version changed, but this prevents the website from showing documentation changes bundled with patch releases.
The change here is to always create a new version in docusaurus, but ensure that for a given X.Y major.minor version we only track the latest patch. We accomplish this by deleting versions we previously created in docusaurus if they only differ by the patch version (i.e. `Z` in `X.Y.Z`)
While Docusaurus provides an npm command to easily create new versions, deleting them has to be done manually using the instructions provided here (which I've codified as a step in the workflow): https://docusaurus.io/docs/versioning#deleting-an-existing-version
Pull Request resolved: https://github.com/facebook/Ax/pull/3898
Test Plan:
- Applied these same changes to my fork in https://github.com/CristianLara/Ax/pull/25
- Then created a new release on my fork to test the workflow
- The final step of the website build failed due to unrelated broken links (fork is quite old): https://github.com/CristianLara/Ax/actions/runs/15542889463/job/43757621741
- But I confirmed that the commit it created properly deleted the related files: https://github.com/CristianLara/Ax/commit/cd54b42773c92330c908b90a814e50077a233f59
Reviewed By: saitcakmak
Differential Revision: D76288339
Pulled By: CristianLara
fbshipit-source-id: 5e1381e79e5165207cf03c61d17b7c7903663c5c
* Fix showing the whole dataframe on 1 point when hovering
Summary: When hovering the mouse, all the data frame were shown on top the same point. Fix the extra comma making the text turning from a list of multiple elements into a tuple with 1 element.
Reviewed By: eonofrey
Differential Revision: D76305434
fbshipit-source-id: ae29995a73493901516a2df30ee8b4691901cbdf
* add attribute for AF on BoTorchGenerator (#3883)
Summary:
Pull Request resolved: https://github.com/facebook/Ax/pull/3883
This is generally useful for inspection and debugging after generation. This isn't saved when we serialize the Generator.
Reviewed By: saitcakmak
Differential Revision: D76052207
fbshipit-source-id: 5245e4b7893ebc801d45e273834bac22ad543660
* Remove SearchSpaceToFloat transform (#3897)
Summary:
Pull Request resolved: https://github.com/facebook/Ax/pull/3897
Removes `SearchSpaceToFloat` transform and replaces its only use case with `SearchSpaceToChoice + ChoiceToNumericChoice + RemoveFixed`.
This transform does not serve a real modeling purpose, and only helps in getting the data into an easy to process format, which can be handled by other existing transforms. Removing it reduces the amount of transform code that we have to maintain.
Reviewed By: esantorella
Differential Revision: D76275126
fbshipit-source-id: de256607609623c9560ed423decd6aa9bea326e0
* Use mini batches in Sobol sensitivity analysis (#3904)
Summary:
Pull Request resolved: https://github.com/facebook/Ax/pull/3904
Very large tensors are used in this function call, which can lead to high peak memory usage. A recent example had `x.shape = 160_000 x 14`, leading to >20gb of memory usage. Using mini-batches here to reduce the peak memory usage. With 4096, we see peak memory usage of ~0.6gb.
Reviewed By: esantorella
Differential Revision: D76369180
fbshipit-source-id: 67e2f10849106f535e5d642d2ec3d467d541a8ca
* Always use model predictions, not empirical best, for best-point (#3894)
Summary:
Pull Request resolved: https://github.com/facebook/Ax/pull/3894
In the short time that Ax benchmarking has supported an inference trace, it has been a perennial source of confusion that there are two different ways to get the "best point" used for the inference trace: the model-recommended best point and the empirical best point. Combined with the "oracle trace," there are three different traces that might be used to evaluate a noisy problem. "Empirical best" -- that is `use_model_predictions_for_best_point=False` -- rarely makes sense to do. It is essentially choosing a point on the basis of noise even if the model is much smarter than that.
This diff:
* Removes the option `use_model_predictions_for_best_point` from `BenchmarkMethod`
Reviewed By: sdaulton
Differential Revision: D76156686
fbshipit-source-id: 00e7448cbbec3bea3f8e9e28b27edd9901a88e15
* Remove IVW transform from Y_trans (#3900)
Summary:
Pull Request resolved: https://github.com/facebook/Ax/pull/3900
`IVW` transform merges repeated observations of a metric in an `ObservationData` object, which requires the experiment to have duplicate observations for the metric for any given `(trial_index, arm_name)`. Based on some offline discussion, this is not a case we expect to encounter in an Ax experiment. This diff removes it from `Y_trans`, which is a subset of the default set of transforms we use across Ax.
Reviewed By: lena-kashtelyan
Differential Revision: D76343356
fbshipit-source-id: e178c0bce8d09bec14de7b9a2711edc5ca8c178b
* Implement StandardizeY.transform_experiment_data (#3889)
Summary:
Pull Request resolved: https://github.com/facebook/Ax/pull/3889
As titled. Supports transforming `ExperimentData` with `StandardizeY` transform. The transform constructor is also updated to extract the mean and std from `ExperimentData`.
Background: As part of the larger refactor, we will be using `ExperimentData` in place of `list[Observation]` within the `Adapter`.
- The transforms will be initialized using `ExperimentData`. The `observations` input to the constructors may be deprecated once the use cases are updated.
- The training data for `Adapter` will be represented with `ExperimentData` and will be transformed using `transform_experiment_data`.
- For misc input / output to various `Adapter` and other methods, the `Observation / ObservationFeatures / ObservationData` objects will remain. To support these, we will retain the existing transform methods that service these objects.
- Since `ExperimentData` is not planned to be used as an output of user facing methods, we do not need to untransform it. We are not planning to implement`untransform_experiment_data`.
Reviewed By: sdaulton
Differential Revision: D76136019
fbshipit-source-id: 23438960d5c3e77ef951fb636d353f7f8a8bae38
* Implement Winsorize.transform_experiment_data (#3890)
Summary:
Pull Request resolved: https://github.com/facebook/Ax/pull/3890
As titled. Supports transforming `ExperimentData` with `Winsorize` transform. The transform constructor is also updated to extract the data used to get cutoffs from `ExperimentData`.
In addition:
- deleted an unused method that is leftover from a previous cleanup.
- Removed unused `observations` input to `derelativize_optimization_config_with_raw_status_quo`.
Background: As part of the larger refactor, we will be using `ExperimentData` in place of `list[Observation]` within the `Adapter`.
- The transforms will be initialized using `ExperimentData`. The `observations` input to the constructors may be deprecated once the use cases are updated.
- The training data for `Adapter` will be represented with `ExperimentData` and will be transformed using `transform_experiment_data`.
- For misc input / output to various `Adapter` and other methods, the `Observation / ObservationFeatures / ObservationData` objects will remain. To support these, we will retain the existing transform methods that service these objects.
- Since `ExperimentData` is not planned to be used as an output of user facing methods, we do not need to untransform it. We are not planning to implement`untransform_experiment_data`.
Reviewed By: esantorella
Differential Revision: D76144125
fbshipit-source-id: e247414e4fd5b00a5c4c970d9f5d151007c18454
* Implement SearchSpaceToChoice.transform_experiment_data (#3896)
Summary:
Pull Request resolved: https://github.com/facebook/Ax/pull/3896
As titled. Supports transforming `ExperimentData` with `SearchSpaceToChoice` transform. The transform constructor is also updated to extract the data used to get the signatures from `ExperimentData`.
Background: As part of the larger refactor, we will be using `ExperimentData` in place of `list[Observation]` within the `Adapter`.
- The transforms will be initialized using `ExperimentData`. The `observations` input to the constructors may be deprecated once the use cases are updated.
- The training data for `Adapter` will be represented with `ExperimentData` and will be transformed using `transform_experiment_data`.
- For misc input / output to various `Adapter` and other methods, the `Observation / ObservationFeatures / ObservationData` objects will remain. To support these, we will retain the existing transform methods that service these objects.
- Since `ExperimentData` is not planned to be used as an output of user facing methods, we do not need to untransform it. We are not planning to implement`untransform_experiment_data`.
Reviewed By: esantorella
Differential Revision: D76273590
fbshipit-source-id: fa0253710b379f003998a57628b4987bebbf6fe5
* Implement TrialAsTask.transform_experiment_data (#3902)
Summary:
Pull Request resolved: https://github.com/facebook/Ax/pull/3902
Supports transforming `ExperimentData` with `TrialAsTask` transform. The transform constructor is also updated to extract the data used to get the trial level mapping from `ExperimentData`.
Background: As part of the larger refactor, we will be using `ExperimentData` in place of `list[Observation]` within the `Adapter`.
- The transforms will be initialized using `ExperimentData`. The `observations` input to the constructors may be deprecated once the use cases are updated.
- The training data for `Adapter` will be represented with `ExperimentData` and will be transformed using `transform_experiment_data`.
- For misc input / output to various `Adapter` and other methods, the `Observation / ObservationFeatures / ObservationData` objects will remain. To support these, we will retain the existing transform methods that service these objects.
- Since `ExperimentData` is not planned to be used as an output of user facing methods, we do not need to untransform it. We are not planning to implement`untransform_experiment_data`.
Reviewed By: sdaulton
Differential Revision: D76348224
fbshipit-source-id: b0fb326ce795e76608a3e960bc3a62dc288f1ea5
* MapKeyToFloat Transform Client API integration tests (#3907)
Summary:
Pull Request resolved: https://github.com/facebook/Ax/pull/3907
This diff introduces integration tests to validate the use of the `MapKeyToFloat` transform within a generation strategy, in conjunction with the Client API. The tests cover typical usage scenarios of the Client API, specifically:
1. **Without Early-Stopping:**
- **Without Progression Information:** Tests the scenario where `client.complete_trial` is called with progression set to `None`.
- **With Progression Information:** Tests the scenario where progression data is provided.
2. **With Early-Stopping:**
- Tests the scenario where intermediate or progressive values are attached using `client.attach_data`, and early stopping is invoked with `client.should_stop_trial_early`.
Reviewed By: saitcakmak
Differential Revision: D76009656
fbshipit-source-id: 94723d9260962b47262ffa7d6e6605475d8d41aa
* PreferenceOptimizationConfig (#3906)
Summary:
Pull Request resolved: https://github.com/facebook/Ax/pull/3906
Enable more PLBO (BO candidate generation part of BOPE) to work with regular MBM and TorchAdapter by supplying pref opt config and set the correct aux PE experiment.
to-do items:
- docstrings
- update sqa storage
Meta: see https://docs.google.com/document/d/1KENayOLCe7-kT85qlOk09AXUHhJXLpqKhsgyVVTwa2Q/edit?tab=t.0#heading=h.6olhm9qnegf3 for more design details and discussions
Reviewed By: saitcakmak
Differential Revision: D71692019
fbshipit-source-id: 3bc735f6b35ad0f66ea3dac10f824eb25f923f65
* Fix transforms used in get_tensor_converter_adapter (#3908)
Summary:
Pull Request resolved: https://github.com/facebook/Ax/pull/3908
In D76275126, this was updated to use `SearchSpaceToChoice` transform, but that doesn't work for all cases since `ChoiceParameter` does not support more than 1000 values.
A simpler solution is to use `MBM_X_trans`, which is part of the default set of transforms Ax uses. If they work as default for any given experiment, they should work well for this utility as well.
Reviewed By: esantorella
Differential Revision: D76431509
fbshipit-source-id: 7020acbaa1f1ca6d1be9127793afef0d55628d39
* Fix unused code in _relativize_values (#3909)
Summary:
Pull Request resolved: https://github.com/facebook/Ax/pull/3909
Noticed that this was returning the means & sems unchanged if the sems were not NaN. Updated the code to fix it.
Reviewed By: esantorella
Differential Revision: D76433080
fbshipit-source-id: c117c8fb601ec51d9e81e57981a0c42852cf25b5
* Fix ArmEffectsPlot x-label overlay + sorting issue (#3901)
Summary:
Pull Request resolved: https://github.com/facebook/Ax/pull/3901
1. Fix repeated arms overly issue. X-mark will be determined by trial-index + arm_names so guaranteed to be unique. Only arm_names will be displayed in x-label.
2. Sort arm_names within trial_index, that 1) non-default arms (not digit + underscore + digit) will be sorted at the front. and 2) default arms (e.g. 0_1, 1_5) will be displayed as 0_1 < 0_5 < 0 _10.
Reviewed By: eonofrey
Differential Revision: D76347874
fbshipit-source-id: 4457d1d56ea1cdc2dab9a1a1311b35b516171b96
* support warped SAAS model in dispatch (#3903)
Summary:
Pull Request resolved: https://github.com/facebook/Ax/pull/3903
See title.
Reviewed By: Balandat, saitcakmak
Differential Revision: D76346097
fbshipit-source-id: 465146e6dbd0be732841711167ec34df780b7d48
* Make model-registry-base easier to make child classes (#3895)
Summary:
If I would like to reuse the machinery of `ModelRegistryBase` I need to manipulate the Ax Global `MODEL_KEY_TO_MODEL_SETUP`. This doesn't feel nice to me and so I propose to make a property in `ModelRegistryBase` to return `MODEL_KEY_TO_MODEL_SETUP` and then in any `MyGenerators` child Enum I can just make my own `MY_MODEL_KEY_TO_MY_MODEL_SETUP` and overwrite that property and in principle everything works and Ax is easier to extend.
Pull Request resolved: https://github.com/facebook/Ax/pull/3895
Reviewed By: paschai
Differential Revision: D76517265
Pulled By: saitcakmak
fbshipit-source-id: e3f24f20ce48ce4d7de16a780f27d14090886cbe
* Fix overlapping Xlabel and legends in ArmEffect (#3915)
Summary:
Pull Request resolved: https://github.com/facebook/Ax/pull/3915
This diff dynamically set legends position using x-ticker's (arm's) length. It assumes the ticker was rotated 90 degrees (vertical format) and ensures there's enough space to avoiding overlaying the x-label `Arm Name` with the legend.
Reviewed By: Cesar-Cardoso
Differential Revision: D76548346
fbshipit-source-id: 75fb901e0ad81b8b0b8adcc86a222c8a9692a6c3
* Use optimize_acqf_mixed_alternating for categorical paramters (#3918)
Summary:
Pull Request resolved: https://github.com/facebook/Ax/pull/3918
Uses the `optimize_acqf_mixed_alternating` optimizer enabled in https://github.com/pytorch/botorch/pull/2866
Reviewed By: saitcakmak
Differential Revision: D76168112
fbshipit-source-id: 6a212f131fc4f6b3fb2517d1608d792be3df99bc
* implement Relativize.transform_experiment_data (#3916)
Summary:
Pull Request resolved: https://github.com/facebook/Ax/pull/3916
Supports transforming `ExperimentData` with `Relativize` transform.
Background: As part of the larger refactor, we will be using `ExperimentData` in place of `list[Observation]` within the `Adapter`.
- The transforms will be initialized using `ExperimentData`. The `observations` input to the constructors may be deprecated once the use cases are updated.
- The training data for `Adapter` will be represented with `ExperimentData` and will be transformed using `transform_experiment_data`.
- For misc input / output to various `Adapter` and other methods, the `Observation / ObservationFeatures / ObservationData` objects will remain. To support these, we will retain the existing transform methods that service these objects.
- Since `ExperimentData` is not planned to be used as an output of user facing methods, we do not need to untransform it. We are not planning to implement`untransform_experiment_data`.
Reviewed By: Balandat
Differential Revision: D76459251
fbshipit-source-id: bd6132c53fc9b1e2cf51030b0201605235c28174
* Use mini batches in predict_from_model (#3912)
Summary:
Pull Request resolved: https://github.com/facebook/Ax/pull/3912
When `X` is large (`66k x d` in a recent example), this function could trigger OOMs. ~~Since it does not utilize cross-covariances between the points, it doesn't need to compute them. This diff updates the method to unsqueeze `n x d`-dim `X` to `n x 1 x d`~~ (unsqueezing actually slows things down and increases memory usage). This diff updates the methods to split `X` to limit `n` to `4096` before evaluating the `posterior`.
Reviewed By: Balandat
Differential Revision: D76514648
fbshipit-source-id: 4fe16a3a7380afbe15ac4e96297cb72ecb253a6b
* Do not unsqueeze X when evaluating posterior in Sobol sensitivity (#3919)
Summary:
Pull Request resolved: https://github.com/facebook/Ax/pull/3919
Profiling in D76514648 showed that unsqueezing `x` before calling `posterior` can actually lead to an increase in runtime & memory usage. Based on these findings, this diff revisits D76369180 to remove the `unsqueeze` from Sobol sensitivity as well.
Reviewed By: esantorella
Differential Revision: D76517910
fbshipit-source-id: 163add8893817d0ba16873573e25bf9cffe65221
* Implement TransformToNewSQ.transform_experiment_data (#3917)
Summary:
Pull Request resolved: https://github.com/facebook/Ax/pull/3917
Pull Request resolved: https://github.com/facebook/Ax/pull/3916
Supports transforming `ExperimentData` with `TransformToNewSQ` transform.
Background: As part of the larger refactor, we will be using `ExperimentData` in place of `list[Observation]` within the `Adapter`.
- The transforms will be initialized using `ExperimentData`. The `observations` input to the constructors may be deprecated once the use cases are updated.
- The training data for `Adapter` will be represented with `ExperimentData` and will be transformed using `transform_experiment_data`.
- For misc input / output to various `Adapter` and other methods, the `Observation / ObservationFeatures / ObservationData` objects will remain. To support these, we will retain the existing transform methods that service these objects.
- Since `ExperimentData` is not planned to be used as an output of user facing methods, we do not need to untransform it. We are not planning to implement`untransform_experiment_data`.
Reviewed By: Balandat
Differential Revision: D76565328
fbshipit-source-id: 5768101fbd12323ade8527102f2c551ad86b0acd
* Remove n_points > 1 option from best-point selector
Summary:
*Context:*
TLDR: I am cleaning up and refactoring best-point functionality. `n_points > 1` is not supported, so it doesn't make sense to refactor it.
* Current state: Currently, the way we get a best point in benchmarking, is opaque because a recommendation is initially written to `GeneratorRun`, then parsed by `BestPointMixin._get_best_trial`, and then some more processing is done to ensure that the point is not None. This is OK for comparing different generation strategies, because it ensures that a best point will always be present. However, it not very helpful for comparing best-point selection itself. The right way to change a best-point recommendation is via `GeneratorRun.best_arm_predictions`, but all the post-processing will make it hard to see the effect of that. We initially wrote `BenchmarkMethod.get_best_parameters` with the expectation that users could subclass `BenchmarkMethod` and override `get_best_parameters` in order to control best-point selection more directly. But subclassing `BenchmarkMethod` is a bad idea that would cause serialization issues, and developing a best-point selector in that way would then require the work be redone to integrate it into Ax.
* Intermediate goal state: Benchmarks simply use best-point predictions from `GeneratorRun.best_arm_predictions`. If this is None, the inference trace will be NaN at that point.
* Ideal end state: Best-point selectors are eventually introduced as an Ax modeling abstraction that can be changed and run independently of candidate generation.
Reviewed By: Balandat
Differential Revision: D76159679
fbshipit-source-id: 914704879009e634cbd189d40665b2c892d39c5c
* Allow BenchmarkMethod.get_best_parameters to return None if there is no best-point recommendation (#3914)
Summary: Pull Request resolved: https://github.com/facebook/Ax/pull/3914
Reviewed By: Balandat
Differential Revision: D76162962
fbshipit-source-id: 3aa7ea377bfb00ffec9862f97b14f955f3f7576e
* Update MetadataToFloat tests to use an actual experiment (#3921)
Summary:
Pull Request resolved: https://github.com/facebook/Ax/pull/3921
Using manually constructed objects (in this case `Observation`) in tests is problematic, since they make it quite difficult to adapt tests to future changes. These objects often include data that cannot be extracted from an actual Ax experiment. In this case, the issue is having multiple `Observation` corresponding to the same `trial_index, arm_name` with different metadata that is not map keys. The `metadata` on `ObservationFeatures` is extracted from `candidate_metadata_by_arm_signature`, which is fixed for a given `trial_index, arm_name` pair (assuming this is not `MapData`, in which case there would be different ones for different map values).
This diff updates `MetadataToFloat` tests to use an `Experiment` and the data that is extracted from that experiment.
Reviewed By: Balandat
Differential Revision: D76611204
fbshipit-source-id: dde27c2754e702382c15e63ff65c069a3b487e6c
* Update MapKeyToFloat transfrom tests to use data from an actual experiment (#3922)
Summary:
Pull Request resolved: https://github.com/facebook/Ax/pull/3922
Updates the tests to (mostly) use data extracted from an actual map data experiment. This will make it easier to extend these tests with `ExperimentData`.
Reviewed By: Balandat
Differential Revision: D76623766
fbshipit-source-id: af8ce8784de79f20fe0dc7c15ebb72972b16f9f3
* Remove a redundant error for MOO in `BenchmarkMethod.get_best_parameters` (#3924)
Summary:
Pull Request resolved: https://github.com/facebook/Ax/pull/3924
`BenchmarkMethod.get_best_parameters` errors when given a MOO opt config, but without that error a very similar one would appear one level deeper in the stack, so there's no need to have one in `get_best_parameters`.
Reviewed By: saitcakmak
Differential Revision: D76618771
fbshipit-source-id: f680b5f77d3ab964875de06be1727373a647be85
* Remove `optimization_config` as argument from `BenchmarkMethod.get_best_parameters` (#3923)
Summary:
Pull Request resolved: https://github.com/facebook/Ax/pull/3923
Context: Passing the optimization config only has an effect if it is different from the optimization config on the experiment. That functionality is only used in one unit test, which I changed to use `clone_with`; in all other cases that the argument was passed, it did nothing.
Reviewed By: saitcakmak
Differential Revision: D76618781
fbshipit-source-id: 56c86fecb31c0f8f351f9179dd9b62e3d3a4a760
* Move `get_best_parameters` off of `BenchmarkMethod` (#3925)
Summary:
Pull Request resolved: https://github.com/facebook/Ax/pull/3925
* Move `get_best_parameters` from `BenchmarkMethod` to `benchmark.py`. No changes made to the content of the function.
* Removed `TestMethods.test_get_best_parameters` -- that just runs a mini-benchmark and then checks that a function has been called; we have other units that check those things. (This test was more substantive in the past, but has already shrunk when previous functionality was removed.)
* Moved a test from `test_benchmark_method.py` to `test_benchmark.py`
Reviewed By: Balandat
Differential Revision: D76620213
fbshipit-source-id: 095704f04200dbaa7a2089a9176766cc90729d2d
* WIP - Validate that all fb experiments have owners when saving (#3511)
Summary: Pull Request resolved: https://github.com/facebook/Ax/pull/3511
Reviewed By: lena-kashtelyan
Differential Revision: D69031030
fbshipit-source-id: 101692b7a13ea4f9ad24f427649ca32840c2f057
* Support ExperimentData in MetadataToFloat & MapKeyToFloat transforms
Summary:
Supports transforming `ExperimentData` with `MetadataToFloat` & `MapKeyToFloat` transforms. The transform constructor is also updated to support extracting the observations for the relevant parameters from `experiment_data`.
For `MapKeyToFloat`, the actual transform is no-op, since we have the map keys in `observation_data` and we can simply extract them from there in `Adapter`.
Background: As part of the larger refactor, we will be using `ExperimentData` in place of `list[Observation]` within the `Adapter`.
- The transforms will be initialized using `ExperimentData`. The `observations` input to the constructors may be deprecated once the use cases are updated.
- The training data for `Adapter` will be represented with `ExperimentData` and will be transformed using `transform_experiment_data`.
- For misc input / output to various `Adapter` and other methods, the `Observation / ObservationFeatures / ObservationData` objects will remain. To support these, we will retain the existing transform methods that service these objects.
- Since `ExperimentData` is not planned to be used as an output of user facing methods, we do not need to untransform it. We are not planning to implement`untransform_experiment_data`.
Reviewed By: ltiao
Differential Revision: D76627256
fbshipit-source-id: 36987dcc52604fcbecfb1a979f47a5d386cdff56
* Add names to SAAS model configs (#3910)
Summary: Pull Request resolved: https://github.com/facebook/Ax/pull/3910
Reviewed By: sdaulton
Differential Revision: D76484600
fbshipit-source-id: 65153882b070dd7a28084493243ef0d64a9af0f1
* Refactor surrogate CV to support data fission (#3911)
Summary:
Pull Request resolved: https://github.com/facebook/Ax/pull/3911
Refactors surrogate cross validation to support data fission. See stacked diff for implementation of that.
Reviewed By: sdaulton
Differential Revision: D74525334
fbshipit-source-id: 71204ebf3251c0964fbb53c2c8c269a6405275d6
* Implement StratifiedStandardizeY.transform_experiment_data (#3920)
Summary:
Pull Request resolved: https://github.com/facebook/Ax/pull/3920
Supports transforming `ExperimentData` with `StratifiedStandardizeY` transform. The transform constructor is also updated to support extracting the strata observations from `experiment_data`.
Background: As part of the larger refactor, we will be using `ExperimentData` in place of `list[Observation]` within the `Adapter`.
- The transforms will be initialized using `ExperimentData`. The `observations` input to the constructors may be deprecated once the use cases are updated.
- The training data for `Adapter` will be represented with `ExperimentData` and will be transformed using `transform_experiment_data`.
- For misc input / output to various `Adapter` and other methods, the `Observation / ObservationFeatures / ObservationData` objects will remain. To support these, we will retain the existing transform methods that service these objects.
- Since `ExperimentData` is not planned to be used as an output of user facing methods, we do not need to untransform it. We are not planning to implement`untransform_experiment_data`.
Reviewed By: sdaulton
Differential Revision: D76606777
fbshipit-source-id: 878e0b6463e69f72b6dac0cc0b2993e7bb4b2b39
* Fix edge cases with no / single point initialization in choose_GS (#3941)
Summary:
Pull Request resolved: https://github.com/facebook/Ax/pull/3941
Choose_GS was adding Center and Sobol nodes even when `initialization_budget=0` was specified. This diff updates it to
- Use MBM only if `initialization_budget=0`.
- Use either Sobol or Center (but not both) if `initialization_budget = 1`, dependending on `initialize_with_center` option.
Reviewed By: Balandat
Differential Revision: D76828041
fbshipit-source-id: 819dad3daf9e421dc046a02804bae681d1683064
* Update Client.predict to return SEM not variance (#3940)
Summary:
Pull Request resolved: https://github.com/facebook/Ax/pull/3940
Updates the code to match the return value in the docstring.
Reviewed By: Balandat
Differential Revision: D76829250
fbshipit-source-id: c5056ed64e1a67e7a55afc9141912f425d1c096b
* Remove unused _raise_deprecation_warning (#3943)
Summary:
Pull Request resolved: https://github.com/facebook/Ax/pull/3943
This is leftover from a recent diff that removed deprecated inputs.
Reviewed By: Balandat
Differential Revision: D76838308
fbshipit-source-id: d5b86e9ff64f0452861dbdad882a9e2b276f93cf
* AnalysisTree refactor (#3882)
Summary:
Pull Request resolved: https://github.com/facebook/Ax/pull/3882
**Tl;dr**: Instead of generating a collection of cards and then grouping/sorting them at display time, we could store them in a tree which captures desired grouping/storing.
Key changes are in ax/analysis/analysis.py; other changes are either downstream type signature changes or test changes.
As we’ve worked on improving the Ax UI user experience wrt. Analyses we keep coming back to this idea of an “AnalysisCard Group”, “master card”, etc. Conceptually we keep wanting to return to this idea of AnalysisCards existing in context together with each other, even though there is no way to capture this in the existing data model.
This diff implements the following changes:
* Introduce new classes
* AnalysisCardBase: holds a name and timestamp
* AnalysisCardGroup: extends AnalysisCardBase, holds children AnalysisCardBases
* AnalysisCard: same as before, but now extends AnalysisCardBase
* Remove metadata used for sorting from AnalysisCard (level, category, attributes)
* See doc for plan for what to do about trial_index
* Chance AnalysisCard.compute to return AnalysisCardBase instead of list[AnalysisCard]
* Move logic necessary for rendering list[AnalysisCard] to AnalysisCardGroup
* Remove logic for sorting list[AnalysisCard] before rendering
* Remove AnalysisBlobAnnotation, which will now be handled on the encoder/decoder level (implemented in next diff)
* Also removed ErrorAnalysisCard since it was just a handle for setting AnalysisBlobAnnotation
* **Removed all callsites to _save_analysis_cards_to_db_if_possible**
Note that this diff removes the ability to store AnalysisCards, which will be re-implemented in a subsequent diff to be landed at the same time as this diff. It also comments out many tests which relied on examining AnalysisCards to function; these will be reworked when we rethink dispatch.
Reviewed By: mgarrard
Differential Revision: D75250277
fbshipit-source-id: 50a35314c38fff3cd754bc7d93a46bebc3c00e8b
* Add storage for AnalysisCardBase (#3938)
Summary:
Pull Request resolved: https://github.com/facebook/Ax/pull/3938
Pull Request resolved: https://github.com/facebook/Ax/pull/3879
As titled. AnalysisCardBase (both single card and group) maps to SQAAnalysisCard, which holds a tree-like structure via a pointer to "parent_id" and sqlalchemy relationships. Important notes:
* blob_annotation, which used to exist on AnalysisCard before this refactor, now exists just as a column in SQAAnalysisCard and is imputed at encode time.
* SQAAnalysisCard also has an "order" col to know which order it belongs in in its parent's children list
* SQLAlchemy does not allow infinite recursive eager loading, so we hard-code looking ~20 layers deep -- this should be more than enough in practice.
Reviewed By: lena-kashtelyan
Differential Revision: D75630615
fbshipit-source-id: bb17bd47ae8f78fb50d157af6c9212771fb0acc5
* Move AnalyisCardBase and children to their own module in ax/analysis (#3878)
Summary:
Pull Request resolved: https://github.com/facebook/Ax/pull/3878
As titled. Suggested by lena-kashtelyan in D75250277
Reviewed By: lena-kashtelyan
Differential Revision: D75976192
fbshipit-source-id: 0cfc95668368930319370174c9109ee45d384988
* Add helper for creating error cards (#3881)
Summary:
Pull Request resolved: https://github.com/facebook/Ax/pull/3881
Add new method `compute_or_error_card` which calls `compute_result` and turns the E into an ErrorCard if necessary. Marginal ergonomic improvement which compounds while working on OverviewAnalysis (next diff).
Reviewed By: mgarrard
Differential Revision: D76044859
fbshipit-source-id: 5e9e47be12b3b0bacc57c18d6a4191fdf61291fc
* Add str helper method on AnalysisCardBase (#3880)
Summary:
Pull Request resolved: https://github.com/facebook/Ax/pull/3880
Adds `hierarchy` method which returns a string representation of the card's nested hierarchy structure. This is useful for debugging and logging.
Ex:
```
Root
Child 1
Grandchild 1
Grandchild 2
Child 2
Grandchild 3
```
Reviewed By: mgarrard
Differential Revision: D76047172
fbshipit-source-id: dc3c2c63056afdda9c37b05ea377a7bb3eb1de22
* OverviewAnalysis (#3937)
Summary:
Pull Request resolved: https://github.com/facebook/Ax/pull/3937
Create a top level analysis which is relevant on ALL Ax experiments. Dispatches to appropriate sub-analyses and combines cards into "Results", "Insights", and "Diagnostics" groups.
Health checks to be added in a following diff.
Diff after will replace all instances of flow-control based analysis dispatch (ie if/else choose_analysis_... methods) with the OverviewAnalysis
Reviewed By: lena-kashtelyan
Differential Revision: D75984682
fbshipit-source-id: 8de03409afd6888141c61386b2afbe149a07f290
* Add HealthChecks to OverviewAnalysis (#3939)
Summary:
Pull Request resolved: https://github.com/facebook/Ax/pull/3939
Pull Request resolved: https://github.com/facebook/Ax/pull/3936
Reviewed By: lena-kashtelyan
Differential Revision: D76369546
fbshipit-source-id: 67736a0c4701357f7a6cc5492aa2d1bca3a3fefb
* Fix for better code formatting (#3944)
Summary:
Pull Request resolved: https://github.com/facebook/Ax/pull/3944
Changed where the failure reason it being set. Looked a bit weird.
Reviewed By: lena-kashtelyan
Differential Revision: D76628017
fbshipit-source-id: 8f45131ca4e2514a7951c835080dcc6b474e8ce9
* Implement Cast.transform_experiment_data (#3935)
Summary:
Pull Request resolved: https://github.com/facebook/Ax/pull/3935
Supports transforming `ExperimentData` with `Cast` transform. For flattening the search space, we use `df.apply` to loop over the rows and update them using existing `HSS.flatten_observation_features` method. This could be worth re-visiting later to see if it could be made more efficient -- or it may not be necessary if we decide to stop flattening altogether based on the results of the internship project.
For `MapKeyToFloat`, the actual transform is no-op, since we have the map keys in `observation_data` and we can simply extract them from there in `Adapter`.
Background: As part of the larger refactor, we will be using `ExperimentData` in place of `list[Observation]` within the `Adapter`.
- The transforms will be initialized using `ExperimentData`. The `observations` input to the constructors may be deprecated once the use cases are updated.
- The training data for `Adapter` will be represented with `ExperimentData` and will be transformed using `transform_experiment_data`.
- For misc input / output to various `Adapter` and other methods, the `Observation / ObservationFeatures / ObservationData` objects will remain. To support these, we will retain the existing transform methods that service these objects.
- Since `ExperimentData` is not planned to be used as an output of user facing methods, we do not need to untransform it. We are not planning to implement`untransform_experiment_data`.
Reviewed By: esantorella
Differential Revision: D76627279
fbshipit-source-id: dcf056745f8a09b148f474f07a88b75206471f20
* Reduce mocking in DiscreteAdapter tests (#3945)
Summary:
Pull Request resolved: https://github.com/facebook/Ax/pull/3945
With these changes, we avoid manually constructed `Observation` objects and adapter-level mocks (mocking only at generator level).
Reviewed By: SebastianAment
Differential Revision: D76916490
fbshipit-source-id: 5543b021d0b4f949554161f52dd3de29159510bc
* Add helper for extracting list[Observation] from ExperimentData (#3948)
Summary:
Pull Request resolved: https://github.com/facebook/Ax/pull/3948
This method will help adapt various Ax functionality that extracts `Adapter.get_experiment_data()` and expects the output to be `list[Observation]`. Having a central method that supports this conversion (possibly applied to a pre-filtered `ExperimentData`) will simplify the refactor quite a bit.
Reviewed By: Balandat
Differential Revision: D77040685
fbshipit-source-id: 365c815a66686409975c3143fb91f936397834b5
* Consolidate DataRequiredError handling in transforms (#3950)
Summary:
Pull Request resolved: https://github.com/facebook/Ax/pull/3950
A number of transforms independently checked for non-empty `observation` or `experiment_data` in their constructors. This diff consolidates that logic on the base `Transform` class and adds a check that `experiment_data` is not empty.
Reviewed By: Balandat
Differential Revision: D77052502
fbshipit-source-id: f4ba09f5b5bc64f7caf18c0cbb2777c54e7fc464
* Add botorch modular registry for custom mll (#3953)
Summary:
Pull Request resolved: https://github.com/facebook/Ax/pull/3953
Adds the ability to use custom mlls in modular botorch with storage by creating a register function the same as exists for Kernels, Models, etc.
Reviewed By: hvarfner
Differential Revision: D77170583
fbshipit-source-id: 39bcfceeeebd834ca64e9791d696bac1c588a642
* relativize by default in ResultsAnalysis and improve tests (#3951)
Summary:
Pull Request resolved: https://github.com/facebook/Ax/pull/3951
see title. This updates the tests to make sure the plots don't error out... This required fixing most of these test cases.
Not all plots support relativize, so we'll want to add that in a follow-up.
Reviewed By: mgarrard
Differential Revision: D77179379
fbshipit-source-id: f1824065cd90c06e0360183c8607ebbbf675ee5e
* fix spelling error (#3952)
Summary:
Pull Request resolved: https://github.com/facebook/Ax/pull/3952
see title
Reviewed By: Cesar-Cardoso
Differential Revision: D77179441
fbshipit-source-id: 0c93e03cb5d5b008da25ad58a0ec21924255790a
* Delete redundant variable assignment (#3955)
Summary:
Pull Request resolved: https://github.com/facebook/Ax/pull/3955
Remove tautological variable assignment
Reviewed By: saitcakmak
Differential Revision: D77228475
fbshipit-source-id: 78fda72c138b3dae0df9b6a4cb9292d24337120b
* Use Ax API in AxSearcher (#3776)
Summary: Pull Request resolved: https://github.com/facebook/Ax/pull/3776
Reviewed By: lena-kashtelyan, saitcakmak
Differential Revision: D67424986
fbshipit-source-id: 4a72e35bdfdb88b9cc54ddaac818d6561265740a
* Implementation of complete_batch_trial and attach_batch_data (#3956)
Summary:
Pull Request resolved: https://github.com/facebook/Ax/pull/3956
This commit includes implementation for complete_batch_trial and attach_batch_data along with their respective unittests.
The `complete_batch_trial` method completes a batch trial and updates the experiment with the provided data. It takes a trial index, raw data (a dictionary mapping arm names to their corresponding outcomes), and an optional progression parameter. The method attaches the data to the trial, updates the trial status, saves the trial to the database if possible, and returns the updated trial status.
The `attach_batch_data` method attaches raw data to a batch trial without completing it. It takes a trial index, raw data (a dictionary mapping arm names to their corresponding outcomes), and an optional progression parameter. The method converts the raw data into a format with progression information, attaches it to the batch trial, and saves the trial to the database if possible. This method is often used in conjunction with `complete_batch_trial` when you want to attach data first and then complete the trial separately.
Reviewed By: mgarrard
Differential Revision: D76319030
fbshipit-source-id: 6b3444b32f5e3c7d42b604777505b063dac5e774
* Code modularization for BatchTrial instanaces (#3957)
Summary:
Pull Request resolved: https://github.com/facebook/Ax/pull/3957
Add a get batch trial method stub that checks if there is an instance of BatchTrial and if so returns it. This reduces repetitive code and also might come in handy when implementing future methods.
Reviewed By: eonofrey
Differential Revision: D76734401
fbshipit-source-id: 97e4f2cc0dad4bf46e20f335c872ff6a124e637a
* Change tensor shapes in unit tests so that stricter equality checks pass (#3959)
Summary:
Pull Request resolved: https://github.com/facebook/Ax/pull/3959
Context: D77042302 makes tesnor equality checks start checking for shape equality (and makes error messages more comprehensible) by moving tests from using `self.assertTrue(torch.allclose(...))` to `self.assertAllClose`, which uses `torch.testing.assert_close` under the hood. The latter is stricter becauase it explicitly checks for shape equality rather than broadcasting, causing test failures.
This PR:
All failures were resolved by changing the "expected" shape to match the "actual" shape. I was able to verify correctness to various degrees.
* In `test_torch_adapter`, a test asserted that qLogNEI.f…
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 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.
Making sure that the torch_device is set to the same as the model in the client.compute_analyses in case CUDA is used in the Modular BoTorch Interface.
Related to the issue #3813
and without the messed-up formatting from #3833, following @Balandat's advice.