Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Botorch with cardinality constraint via sampling #301
base: main
Are you sure you want to change the base?
Botorch with cardinality constraint via sampling #301
Changes from 57 commits
d811240
da813f5
adf5cc2
e69ceff
2270350
6483e4b
ae919d4
9ab8fda
c3831c7
2f49f5a
d46fd60
293e2ef
f8d0713
76b5d72
5c92079
f07a452
c5b014d
306c9d2
7687e39
66c3278
a1d11e7
22fd942
120d717
e6248b5
e0508b9
04d89d1
102ef07
e357c95
3d72f04
ed8054b
756bb09
b0c422e
ddabcf5
b8d24d7
492bb3b
584d8d9
5744e31
e4afdcc
788a5ba
046a8e2
7aac4d3
3c829d0
bc697c0
b9038b8
c2c8b99
a721f21
e84dda9
fa13267
7aa7c3f
cfdf1e3
e3c6620
b85924f
22b19f9
b0dc037
3fa8b02
35825b8
3e275e4
62f0ed6
9af846b
10e0812
142b1ec
04f145c
55a7ba3
78b115f
68045a7
e6e2e97
a30b009
983b1a9
bddab62
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
I think these as well as the utilities noted below are not user-facing, are they? In that case, I do not think that it is necessary to include them in the CHANGELOG
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.
would reutilzie the list
param_names_continuous
from aboveThere 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.
I'm not sure if that's too helpful, since it contains only the names. Or what exactly do you suggest?
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.
.get_parameters_by_name
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.
@AdrianSosic I didn't find the method mentioned above. In general
.get_parameters_by_name
can be really handy at several places. Let me know pls, if it is already implemented somewhere or if it should be added in this PR (I can add 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.
sry for my confusion, the method exists in this PR https://github.com/emdgroup/baybe/pull/291/files#diff-9b02c8d8e9e86b086ea306806ebc5e47435ac5045557f8eb138a7be22c3cb0e8R461 which is however on hold
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.
@Scienfitz The PR above has been merged. I used
.get_parameters_by_name
somewhere else, but not here. It is a method ofSubspaceContinuous
and we do not have thesubspace_continuous
object here. In addition, we need the typeNumericalContinuousParameter
inparams_continuous
. I prefer to keep it in this way unless there is a strong opinion against 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.
Insonsistencies regarding the use of
near_zero
andnear-zero
in this docstring.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.
It is renamed to
zeros
in the updated version. Resolve it for now. Ifnear_zero
is preferred, I'm open to rename it back.