Skip to content
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

Fixed slashes in ////zeta in BOPE tutorial #1777

Closed
wants to merge 1 commit into from

Conversation

esantorella
Copy link
Member

Motivation

A pandas update broke the CI. This fixes it.

Have you read the Contributing Guidelines on pull requests?

Yes

Test Plan

Repro'd the error locally and confirmed that this fixes it. The CI should pass now.

@facebook-github-bot facebook-github-bot added the CLA Signed Do not delete this pull request or issue due to inactivity. label Apr 4, 2023
Copy link
Contributor

@Balandat Balandat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

excellent, thanks for the quick fix

@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 Apr 4, 2023

Codecov Report

Merging #1777 (aaa6b15) into main (ae7ceb9) will not change coverage.
The diff coverage is n/a.

❗ Current head aaa6b15 differs from pull request most recent head d3b8138. Consider uploading reports for the commit d3b8138 to get more accurate results

@@            Coverage Diff            @@
##              main     #1777   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          170       170           
  Lines        14760     14760           
=========================================
  Hits         14760     14760           

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@facebook-github-bot
Copy link
Contributor

@esantorella merged this pull request in 7b48c18.

facebook-github-bot pushed a commit that referenced this pull request Jun 28, 2023
…1907)

Summary:
## Motivation

* A Pandas upgrade changed the way that Greek letters needed to be escaped for plotting, so in #1777 we changed "\\\\zeta" to "\\zeta". However, this caused failures for those using an older version of Pandas. Therefore, we will simply represent it as "zeta" for plotting.
* Since only one replication is run, there is no need to compute an "sem", since it is always NaN over one data point. So I removed that part of the analysis and plotting logic.

### Have you read the [Contributing Guidelines on pull requests](https://github.com/pytorch/botorch/blob/main/CONTRIBUTING.md#pull-requests)?

Yes

Pull Request resolved: #1907

Test Plan:
I ran the notebook.

## Related PRs

#1777

Reviewed By: Balandat

Differential Revision: D47096998

Pulled By: esantorella

fbshipit-source-id: 8739c2c5685ed76e41258e46316a69dcc5527bc9
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. Merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants