-
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
batch constraints for DoE and __call__ for interpointEqualityConstraint #322
batch constraints for DoE and __call__ for interpointEqualityConstraint #322
Conversation
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.
Looks good to me, what do you think, should we add another test in test_design.py
in which we actually test that the DoE module is able to create such designs?
], | ||
) | ||
def test_constraints_call(constraints, num_candidates): | ||
candidates = inputs.sample(num_candidates, SamplingMethodEnum.UNIFORM) | ||
returned = constraints(candidates) | ||
assert returned.shape == (num_candidates, len(constraints)) | ||
|
||
num_rows = 0 |
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.
Are we anywhere testing that the call method returns the correct values? We need to urgently refactor our tests :D
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.
At least I haven't found them :D But added one testing if the design generation works with batch constraints.
Weird that the test |
Just ignore it, this is due to randomness ... |
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.
Looks good to me. @KappatC ok for you to merge?
x1 = np.round(result["x1"].values, 6) | ||
|
||
assert 0 in x1 and 1 in x1 | ||
for i in range(10): |
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 just call domain.validate_candidates(result, only_inputs=True)
in the test. This is also doing the check.
yes, please go ahead |
This PR adds a call method for batch constraints as discussed in #313 and adds support for batch constraints /
InterpointEqualityConstraint
in DoE as discussed in #310. Closes #310