Skip to content

Conversation

@esantorella
Copy link
Contributor

Motivation

Unit tests were producing a lot of warnings about taking standard deviations across fewer than 2 observations, and it was not clear to me if these warnings were legitimate in context.

  • For checking the standardization of input data, no longer check the standard deviation if there is just one observation.
  • For the standardize input transform, explicitly set standard deviations to 1 when there is only one observation. This actually matches the legacy behavior, but previously it wasn't clear because the standard deviation would become NaN before being corrected to 1.
  • Error on attempting to standardize 0 observations. This never worked so now it is more clear.

Test Plan

Added units

Related PRs

@esantorella esantorella self-assigned this May 29, 2024
@facebook-github-bot facebook-github-bot added the CLA Signed Do not delete this pull request or issue due to inactivity. label May 29, 2024
@facebook-github-bot
Copy link
Contributor

@esantorella has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@codecov
Copy link

codecov bot commented May 29, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.98%. Comparing base (d753706) to head (cdcc6c0).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2357   +/-   ##
=======================================
  Coverage   99.98%   99.98%           
=======================================
  Files         190      190           
  Lines       16590    16603   +13     
=======================================
+ Hits        16587    16600   +13     
  Misses          3        3           

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

@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D57931412

esantorella added a commit to esantorella/botorch that referenced this pull request Jun 11, 2024
… silence some unit test warnings (meta-pytorch#2357)

Summary:
## Motivation

Unit tests were producing a lot of warnings about taking standard deviations across fewer than 2 observations, and it was not clear to me if these warnings were legitimate in context.
* For checking the standardization of input data, no longer check the standard deviation if there is just one observation.
* For the standardize input transform, explicitly set standard deviations to 1 when there is only one observation. This actually matches the legacy behavior, but previously it wasn't clear because the standard deviation would become NaN before being corrected to 1.
* Error on attempting to standardize 0 observations. This never worked so now it is more clear.

Pull Request resolved: meta-pytorch#2357

Test Plan:
Added units

## Related PRs

Reviewed By: Balandat

Differential Revision: D57931412

Pulled By: esantorella
… silence some unit test warnings (meta-pytorch#2357)

Summary:
## Motivation

Unit tests were producing a lot of warnings about taking standard deviations across fewer than 2 observations, and it was not clear to me if these warnings were legitimate in context.
* For checking the standardization of input data, no longer check the standard deviation if there is just one observation.
* For the standardize input transform, explicitly set standard deviations to 1 when there is only one observation. This actually matches the legacy behavior, but previously it wasn't clear because the standard deviation would become NaN before being corrected to 1.
* Error on attempting to standardize 0 observations. This never worked so now it is more clear.

Pull Request resolved: meta-pytorch#2357

Test Plan:
Added units

## Related PRs

Reviewed By: Balandat

Differential Revision: D57931412

Pulled By: esantorella
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D57931412

@facebook-github-bot
Copy link
Contributor

@esantorella merged this pull request in f79d608.

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. fb-exported Merged

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants