Skip to content

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.

Sign up for GitHub

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

Closed
wants to merge 1 commit into from
Closed

Point to example for constraints def in optimize_acqf docstring #1900

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions botorch/optim/optimize.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Contributor

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.

Copy link

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

Copy link
Contributor Author

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 :)

Copy link

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
image

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.

Copy link

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

applying the same constraint to each candidate in the batch,
`indices` should be a 1-d tensor. For inter-point constraints,
`indices` must be a 2-d Tensor, where in each row `indices[i] =
Expand All @@ -455,7 +456,8 @@ def optimize_acqf(
the `l_i`-th feature of that element.
equality_constraints: A list of tuples (indices, coefficients, rhs),
with each tuple encoding an equality constraint of the form
`\sum_i (X[indices[i]] * coefficients[i]) = rhs`
`\sum_i (X[indices[i]] * coefficients[i]) = rhs`. See the docstring of
`make_scipy_linear_constraints` for an example.
nonlinear_inequality_constraints: A list of callables with that represent
non-linear inequality constraints of the form `callable(x) >= 0`. Each
callable is expected to take a `(num_restarts) x q x d`-dim tensor as an
Expand Down