search

josejimenezluna / pyGPGO

Bayesian optimization for Python
http://pygpgo.readthedocs.io
MIT License
241 stars 62 forks source link

Fixing flaky test #34

Open crawlingcub opened 3 years ago

crawlingcub commented 3 years ago

This is a fix for the issue #31. The test was failing when the seed was not set. We observed that increase number of trials to 20 decreases the flakiness to ~0%. To enable this change, we added a parameter to the fit function in pyGPGO/surrogates/tStudentProcess.py. Please let us know if this change seems reasonable.

We will be happy to incorporate any other suggestions. Thanks!

If you think this is useful, we can also look into other tests that were failing.

josejimenezluna commented 3 years ago

Hi @crawlingcub, many thanks for this PR.

I don't think changes in the surrogate API are really necessary in this case (e.g. changes to tStudentProcess.py), as the optHyp method is set to 5 trials by default.

https://github.com/josejimenezluna/pyGPGO/blob/master/pyGPGO/surrogates/tStudentProcess.py#L116

I'd be happy to accept the proposed change to 20 trials in the test file, however.

Would this be ok?

Cheers, Jose

crawlingcub commented 3 years ago

Hi @josejimenezluna , sorry i edited the wrong test. The "fit" function in tStudentProcess.py doesn't currently accept a trials argument. So we cant pass it from the test as well. Hence, I added that parameter. Does that make sense?

crawlingcub commented 3 years ago

@josejimenezluna Just to add, if you have any suggestions about how to refactor the test in a different way, I will be happy to incorporate that. Thanks!