Skip to content

Conversation

@esantorella
Copy link
Contributor

NOTE: I am making several PRs each expanding on model documentation. I am trying to identify the best subject-matter experts for each as a reviewer. In this case this is @dme65 , especially for the fully Bayesian models; let me know if someone else should review the variational models.

this is the last docstring PR!

Motivation

Make it easier for people to understand what the different models do and figure out which one is right for their use case

Have you read the Contributing Guidelines on pull requests?

Yes

Test Plan

  • Asked relevant experts for more info where existing documentation was unclear
  • Built the docs and made sure they rendered okay

Related PRs

#1327, #1328 , #1329

@esantorella esantorella requested a review from dme65 July 29, 2022 23:14
@esantorella esantorella self-assigned this Jul 29, 2022
@facebook-github-bot facebook-github-bot added the CLA Signed Do not delete this pull request or issue due to inactivity. label Jul 29, 2022
@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 Jul 29, 2022

Codecov Report

Merging #1330 (985cfbc) into main (f8746ea) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #1330   +/-   ##
=======================================
  Coverage   99.99%   99.99%           
=======================================
  Files         122      122           
  Lines       10778    10778           
=======================================
  Hits        10777    10777           
  Misses          1        1           
Impacted Files Coverage Δ
botorch/models/approximate_gp.py 100.00% <ø> (ø)
botorch/models/fully_bayesian.py 100.00% <ø> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us.

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

Copy link
Contributor

@dme65 dme65 left a comment

Choose a reason for hiding this comment

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

Thanks for improving our documentation! I rewrote a few sentences, but lgtm otherwise!

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

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants