Skip to content

fix: Numerical tolerance for filtering of X_avoid#3301

Open
atinary-lbrey wants to merge 3 commits into
meta-pytorch:mainfrom
atinary-lbrey:lbrey/fix-numerical-error-X-avoid
Open

fix: Numerical tolerance for filtering of X_avoid#3301
atinary-lbrey wants to merge 3 commits into
meta-pytorch:mainfrom
atinary-lbrey:lbrey/fix-numerical-error-X-avoid

Conversation

@atinary-lbrey
Copy link
Copy Markdown

Motivation

Realized there was some numerical instabilities with hard equality check when filtering X_avoid points from choices in optimize_discrete*

Have you read the Contributing Guidelines on pull requests?

Yes

Test Plan

(Write your test plan here. If you changed any code, please provide us with clear instructions on how you verified your changes work. Bonus points for screenshots and videos!)
Added a test case in the TestOptimizeDiscrete by adding noise up to hard tolerance (1e-8) and checking that the behavior was the same as in the exact case.

Related PRs

(If this PR adds or changes functionality, please take some time to update the docs at https://github.com/meta-pytorch/botorch, and link to your PR here.)

@meta-cla meta-cla Bot added the CLA Signed Do not delete this pull request or issue due to inactivity. label May 4, 2026
@atinary-lbrey atinary-lbrey marked this pull request as ready for review May 4, 2026 16:54
@codecov
Copy link
Copy Markdown

codecov Bot commented May 18, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.98%. Comparing base (83f2c4c) to head (cde654a).
⚠️ Report is 3 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #3301   +/-   ##
=======================================
  Coverage   99.98%   99.98%           
=======================================
  Files         225      225           
  Lines       22289    22289           
=======================================
  Hits        22285    22285           
  Misses          4        4           

☔ 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.

@saitcakmak
Copy link
Copy Markdown
Contributor

Hi @atinary-lbrey. Do you have examples of cases where the exact equality checks cause issues? It is a bit surprising that exact equality checks against candidates (presumably) produced by the discrete optimizer would fail. Do you provide X_avoid separately from an external source?

Overall, this seems like a reasonable solution at first but I have my concerns. These broadly apply to any discrete optimizer. isclose uses atol=1e-8, rtol=1e-5 by default. Both of these are problematic in two extremes, which are both unlikely to happen.

  • atol=1e-8: If the parameter magnitudes are extremely small, they may get equal by default. Such small values are likely to cause numerical issues elsewhere as well, so they're not recommended.
  • rtol=1e-5: If a parameter spans a small range with a large common factor, like [1_000_001, 1_000_002, ...], these will also count as equal. I think this is also very unlikely.
    If we're using isequal with these tolerances, it'd be good to flag these edge cases in the docstring, so that the underlying assumptions are clearly communicated to the user. One step further would be to validate that the provided search spaces do not violate these assumptions when the optimizer is called, and error out if an unsupported search space is provided.

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.

2 participants