Skip to content
This repository has been archived by the owner on Feb 23, 2023. It is now read-only.

Pass constraints to anchor point generator #150

Merged
merged 2 commits into from
Feb 28, 2018

Conversation

LEleonora
Copy link
Contributor

Currently the constraints are not being respected in the anchor point
generator. This could lead to infeasible points being selected as
anchor points. To fix this simply pass the commits to the design space
object that is used for creating the anchor points.

Currently the constraints are not being respected in the anchor point
generator. This could lead to infeasible points being selected as
anchor points. To fix this simply pass the commits to the design space
object that is used for creating the anchor points.
@javiergonzalezh
Copy link
Member

Thanks for the PR! this is certainly a good catch!!! However, some of the tests are not passing yet. Could you please investigate that before we can merge? It would be great to add this to the library.

The update to the design space in the anchor point generator resulted in
infeasible points being discovered when generating the initial samples. This
lead to diverging code path. Updating the expected test output to reflect this.
@LEleonora
Copy link
Contributor Author

Hi, I checked that the results are feasible and everything is looking good at this end. Furthermore, I checked where the two code paths diverge.

The only part of the code where the modified space is being used is the call to initial_design and subsequently RandomDesign. There, a few infeasible points are being generated by the call to get_samples_without_constraints. In the patched version of the code, get_samples_with_constraints catches this and generates replacements. Naturally, this leads to different initial designs and a different random generator state down the road.

To sum up, it is probably best to replace the test case with the new output. I added a commit to do just that.

@codecov-io
Copy link

codecov-io commented Jan 15, 2018

Codecov Report

Merging #150 into master will increase coverage by <.01%.
The diff coverage is 88.88%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #150      +/-   ##
==========================================
+ Coverage   76.74%   76.75%   +<.01%     
==========================================
  Files          92       92              
  Lines        4162     4168       +6     
==========================================
+ Hits         3194     3199       +5     
- Misses        968      969       +1
Impacted Files Coverage Δ
...PyOpt/testing/functional_tests/test_constraints.py 97.14% <ø> (ø) ⬆️
GPyOpt/core/task/space.py 88.03% <100%> (+0.23%) ⬆️
GPyOpt/optimization/anchor_points_generator.py 96.29% <100%> (ø) ⬆️
GPyOpt/methods/bayesian_optimization.py 87.77% <66.66%> (-0.86%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 32b83c5...50e93fd. Read the comment docs.

@LEleonora LEleonora force-pushed the feasible_anchor_points branch from 3e6131f to 50e93fd Compare January 25, 2018 08:42
@apaleyes
Copy link
Collaborator

That looks good, let's merge.

@apaleyes apaleyes merged commit 693f8fb into SheffieldML:master Feb 28, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants