search

AnotherSamWilson / ParBayesianOptimization

Parallelizable Bayesian Optimization in R
107 stars 18 forks source link

Fix #52: removing `setorder` due to reproducibility-issues #53

Closed kapsner closed 2 years ago

kapsner commented 2 years ago

This PR fixes #52 by removing setorder which is currently applied to the LocalOptims object and leads to inconsistent results on different hosts when ties occur in the sorting criteria (gpUtility).

AnotherSamWilson commented 2 years ago

Does this pass a local R CMD check? It looks like travis CI fails because xgboost isn't available... What a pain.

kapsner commented 2 years ago

I only did devtools::test() and it was was passed locally.

AnotherSamWilson commented 2 years ago

Okay I'll pull this down and run the CMD checks - if it's just Travis that's the problem we should be good to go.

kapsner commented 2 years ago

Thx. Let me know the outcome pls, in case of other issues I will go ahead and fix them!

kapsner commented 2 years ago

Hi @AnotherSamWilson , I have finally managed to verify the functioning of this branch again: running rcmdcheck::rcmdcheck(args = "--as-cran") resulted in the following notes:

❯ checking for hidden files and directories ... NOTE Found the following hidden files and directories: .travis.yml These were most likely included in error. See section ‘Package structure’ in the ‘Writing R Extensions’ manual.

CRAN-pack does not know about .travis.yml

❯ checking top-level files ... NOTE Non-standard file/directory found at top level: ‘cran-comments.md’

❯ checking R code for possible problems ... NOTE getNextParameters: no visible binding for global variable ‘.SD’ Undefined global functions or variables: .SD

Found if() conditions comparing class() to string: File ‘ParBayesianOptimization/R/SmallFuncs.R’: if (class(bounds) != "list") ... File ‘ParBayesianOptimization/R/addIterations.R’: if (class(optObj) != "bayesOpt") ... File ‘ParBayesianOptimization/R/bayesOpt.R’: if (class(Result) != "list") ... File ‘ParBayesianOptimization/R/changeSaveFile.R’: if (class(optObj) != "bayesOpt") ... File ‘ParBayesianOptimization/R/updateGP.R’: if (class(sgp) == "stopEarlyMsg") ... Use inherits() (or maybe is()) instead.

The latest commit fixes these issues. Now the results are

0 errors ✔ | 0 warnings ✔ | 0 notes ✔

AnotherSamWilson commented 2 years ago

@kapsner Sorry for the late reply, it's crunch time at work right now. Would you like to be a maintainer of this project? I'm afraid I can't give this project my full attention between work and my other open-source projects.

kapsner commented 2 years ago

Hello @AnotherSamWilson , thank you very much for the offer to become co-maintainer of this project. Since I also maintain several open-source projects myself and my time is also very limited, I am honestly a bit cautious with a commitment. How would you imagine such a co-maintainment?

kapsner commented 2 years ago

Hi @AnotherSamWilson , have you in the meantime been able to have a look at this? Can this PR be merged or is there yet missing something?