Clamp to 0 to avoid sqrt of negative numbers in mvn_hellinger_distance

[Jack-Sandberg]

Motivation

When trying out ScoreBO, I noticed that it tended to explore excessively. I found that the acquisition function occasionally contained NaNs that originated from the mvn_hellinger_distance function. When computing 1 - x.exp() for small x, the result is sometimes negative due to floating point inaccuriacies which causes the sqrt to output NaN.

Have you read the Contributing Guidelines on pull requests?

Yes.

Test Plan

The change is rather minor but I've added a test to test_community/utils/test_stat_dist.py.

[meta-cla]

Hi @Jack-Sandberg!

Thank you for your pull request and welcome to our community.

Action Required

In order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you.

Process

In order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with CLA signed. The tagging process may take up to 1 hour after signing. Please give it that time before contacting us about it.

If you have received this in error or have any questions, please contact us at [email protected]. Thanks!

[hvarfner]

Hi @Jack-Sandberg ,

Thanks for looking into this, and for taking the time to put this together. Out of curiosity, do you know if there were certain situations where this was more likely to happen? (e.g. earlier/later iterations, high/low noise)

Nonetheless, the fix looks good. Thanks!

[hvarfner]

@Jack-Sandberg Pease make sure to go through the Contributor License Agreement so that the PR can be merged!

[meta-cla]

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks!

[Jack-Sandberg]

Hi @hvarfner,

The CLA should have been signed now.

I was adapting ScoreBO to my own setting where I use discrete arms and discrete lengthscale values. I did not use standardize input or standardize output because my data is from a GP that matches the prior assumptions and should be reasonably well-behaved. The GP has a variance of 1 and the noise has a variance of 0.25^2.

Currently, the NaNs show up in the second iteration at the location of the first data point. With discrete arms, the acquisition function must be evaluated at the exact same location as the data has been collected (and potentially where the sampled optimas $x^*$ lie) which might explain why the parameters in the test case are almost identical. I have not investigated the behavior in continuous settings.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.99%. Comparing base (645d9e5) to head (29f56b0).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #3109   +/-   ##
=======================================
  Coverage   99.99%   99.99%           
=======================================
  Files         219      219           
  Lines       21174    21174           
=======================================
  Hits        21172    21172           
  Misses          2        2           

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

[meta-codesync]

@hvarfner has imported this pull request. If you are a Meta employee, you can view this in D88863196.

[meta-codesync]

@hvarfner merged this pull request in 53dbaa9.