-
Notifications
You must be signed in to change notification settings - Fork 23
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
Efficient NChooseKs in BO #156
Conversation
I think
Since there is no implementation for
|
Thanks for spotting this, honestly I have no idea why it is not working as intended. What do you mean with |
I think I didn't explain what I meant well. I think the problem is that each of the function refers to
However, calling both functions
shows that both functions will return 1, since both functions will access
I haven't looked it up, but to my understanding this is because the there is only one
|
You are completely correct. I overlooked this, when coding this up. Many thanks for spotting this! I just fixed it and added some tests for it. Concerning your suggestion of implementing this narrow gaussian approach also directly in the the But I have at least one question: why do you need the Best, Johannes |
Sure, np! Ok, then I will proceed as planned with the NChooseK constraint. The Side note: for every NChooseK constraint that is not ill-posed at most one of the two constraint evaluations will give you a positive number. So basically the ReLU just prevents unwanted behaviour due to cancellation of negative and positive terms if one of the |
This makes absolutely sense. Nice idea, I had to use the other convention, since botorch is working with this one. Have you already tried if |
This PR adds the following things:
This PR depends on this PR in botorch: pytorch/botorch#1779. Tests will only able to run when the thing is merged in botorch,