Skip to content

Probabilistic Reparameterization Part 2 #2882

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 19 commits into
base: main
Choose a base branch
from

Conversation

TobyBoyne
Copy link
Contributor

Motivation

Probabilistic Reparameterization is a technique for optimizing mixed feature spaces, proposed in [1]. There is an existing pull request (#1533), which requires resolving merge conflicts, and writing tests. This PR continues that work.

[1] Samuel Daulton et al. Bayesian Optimization over Discrete and Mixed Spaces via Probabilistic Reparameterization, NeurIPS '22

Have you read the Contributing Guidelines on pull requests?

Yes

Test Plan

I have currently written tests that optimize the acquisition function. I will also write more fine grained tests: specifically, testing the structure of the chained transforms generated in factory.py, and testing the new transforms.

Related PRs

This continues the work in #1533. The tutorial is in #1534, which also would need updating.

sdaulton and others added 13 commits February 14, 2023 16:31
Summary:
Pull Request resolved: pytorch#1532

Add a wrapper for modifying inputs/outputs. This is useful for not only probabilistic reparameterization, but will also simplify other integrated AFs (e.g. MCMC) as well as fixed feature AFs and things like prior-guided AFs

Differential Revision: https://internalfb.com/D41629186

fbshipit-source-id: c2d3b339edf44a3167804b095d213b3ba98b5e13
Summary: Creates a new helper method for checking both if a given AF is an instance of a class or if the given AF wraps a base AF that is an instance of a class

Differential Revision: D43127722

fbshipit-source-id: 9f5f31b991f15f2b32931f1b9625422c7907495d
Summary:
Pull Request resolved: pytorch#1533

Probabilistic reparameterization

Differential Revision: D41629217

fbshipit-source-id: a6067c73ce534daf6f6a180fc49720f305827d58
@facebook-github-bot facebook-github-bot added the CLA Signed Do not delete this pull request or issue due to inactivity. label Jun 16, 2025
@TobyBoyne
Copy link
Contributor Author

Currently, the test comparing the Analytic vs MC methods for PR do not give the same (or close to the same) results, which doesn't seem correct to me. I will keep investigating, I imagine it is an issue with my code since the results in the paper seem to be fairly consistent between Analytic and MC (it seems to be that the Analytic approach is giving 0 acqf value everywhere). If anyone does spot any mistakes, please let me know!

For reference, the MC method in this test does give similar results to completely enumerating the mixed features.

@esantorella
Copy link
Member

cc @sdaulton

@eytan
Copy link
Contributor

eytan commented Jun 16, 2025

Great to see you picking this up @TobyBoyne—thanks so much for your contributions to the project!

It'll be cool to see PR working with BARK—seems like a natural way to get around using MIP and a good use of those MCMC samples!

@TobyBoyne
Copy link
Contributor Author

TobyBoyne commented Jun 18, 2025

Some questions I've found while working on this:

  1. I have included the tests for the *PRInputTransform with the other PR tests. Should they be moved to test/models/transform/input.py? Similarly, should these transforms even be with the other InputTransforms, since they are highly specific to PR?
  2. Flexibility in dimension position - we currently assume/enforce that the dimensions are ordered continuous->discrete->categorical. Should this be relaxed, or is it okay to leave for the sake of cleaner code?
  3. Flexibility in encoding - PR currently only supports one-hot encoding for categoricals. In Support categoricals in alternating optimization #2866, I assumed that categoricals were encoded ordinally. Is there a canonical/preferred encoding for categoricals? Should both be supported?

@TobyBoyne
Copy link
Contributor Author

Analytic vs MC methods for PR do not give the same (or close to the same) results

For anyone interested, this was due to PR implicitly assuming that integer idxs are in the rightmost columns (after continuous), whereas the MixedAckley benchmark places them in the leftmost columns.

Copy link

codecov bot commented Jun 24, 2025

Codecov Report

Attention: Patch coverage is 89.61353% with 43 lines in your changes missing coverage. Please review.

Project coverage is 99.78%. Comparing base (13444be) to head (5a1b0fc).
Report is 7 commits behind head on main.

Files with missing lines Patch % Lines
...ch/acquisition/probabilistic_reparameterization.py 80.32% 36 Missing ⚠️
botorch/models/transforms/input.py 95.90% 7 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##              main    #2882      +/-   ##
===========================================
- Coverage   100.00%   99.78%   -0.22%     
===========================================
  Files          211      213       +2     
  Lines        19512    19956     +444     
===========================================
+ Hits         19512    19913     +401     
- Misses           0       43      +43     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@sdaulton
Copy link
Contributor

I have included the tests for the *PRInputTransform with the other PR tests. Should they be moved to test/models/transform/input.py?

It seems fine to leave them with the other PR tests.

Similarly, should these transforms even be with the other InputTransforms, since they are highly specific to PR?

I think it makes sense to put them in a separate file (perhaps acquisition/probabilistic_reparameterization.py), since they are only used via AnalyticProbabilisticReparameterization and MCProbabilisticReparameterization.

Flexibility in dimension position - we currently assume/enforce that the dimensions are ordered continuous->discrete->categorical. Should this be relaxed, or is it okay to leave for the sake of cleaner code?

Ideally this would be relaxed, but I think we can leave that as a TODO for now if it adds to much complexity. If we do leave the ordering assumption, let's make sure we raise exceptions if the ordering is violated.

Flexibility in encoding - PR currently only supports one-hot encoding for categoricals. In #2866, I assumed that categoricals were encoded ordinally. Is there a canonical/preferred encoding for categoricals? Should both be supported?

We should only be using one-hot encoding (or potentially embeddings) for categoricals. The ordinal probabilistic reparameterization proposed in the paper really only makes sense if there is a inherent ordering to the categorical values (even if the GP is using say a categorical kernel), since to move from theta=3->theta=1 you would have to pass through 2, if an ordinal representation is used. One-hot is much more intuitive and avoids that scenario.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed Do not delete this pull request or issue due to inactivity.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants