-
Notifications
You must be signed in to change notification settings - Fork 427
Point to example for constraints def in optimize_acqf docstring #1900
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
Conversation
Summary: In response to pytorch#1899 Differential Revision: D46900427 fbshipit-source-id: 5c903e9aa876095554ecc5167bdcd89ba9d082fb
This pull request was exported from Phabricator. Differential Revision: D46900427 |
Codecov Report
@@ Coverage Diff @@
## main #1900 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 173 173
Lines 15194 15194
=========================================
Hits 15194 15194
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
@@ -446,7 +446,8 @@ def optimize_acqf( | |||
inequality_constraints: A list of tuples (indices, coefficients, rhs), | |||
with each tuple encoding an inequality constraint of the form | |||
`\sum_i (X[indices[i]] * coefficients[i]) >= rhs`. `indices` and | |||
`coefficients` should be torch tensors. When q=1, or when | |||
`coefficients` should be torch tensors. See the docstring of | |||
`make_scipy_linear_constraints` for an example. When q=1, or when |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is definitely an improvement but what is make_scipy_linear_constraints
and where does one find it? It is not imported in this file, so the user would have to search the repo to find where it is defined.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
google botorch make_scipy_linear_constraints. Also, look at this post I made when trying to understand how it works
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fair point, this could definitely improved upon, but I wanted to get something out that's actionable (search/googleable) right away rather than putting this on a backlog list and then forget about it :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding the
see the docstring of
make_scipy_linear_constraints
for an example
allows for googling
I just searched for it and the 3rd option was the charm, giving me this link, where you can find, with a bit more search, this part of the page
I can see it's not the best option, but at least it allows the user to understand what's going on behind the scenes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You could also link to this question I made in stackoverflow in the docs. IT's just my rambling and understanding of how the process is working, but could be useful
This pull request has been merged in 46d4c51. |
Summary: In response to #1899
Differential Revision: D46900427