h2oai / h2o-3

H2O is an Open Source, Distributed, Fast & Scalable Machine Learning Platform: Deep Learning, Gradient Boosting (GBM) & XGBoost, Random Forest, Generalized Linear Modeling (GLM with Elastic Net), K-Means, PCA, Generalized Additive Models (GAM), RuleFit, Support Vector Machine (SVM), Stacked Ensembles, Automatic Machine Learning (AutoML), etc.
http://h2o.ai
Apache License 2.0
6.87k stars 2k forks source link

Parallel Grid search does not support case when `max_runtime_secs()` is 0 ( meaning unlimited by time search) #8519

Closed exalate-issue-sync[bot] closed 1 year ago

exalate-issue-sync[bot] commented 1 year ago

Issue 1: Existing tests for GridSearch were only checking against CartesianHyperSpaceWalker and corresponding CartesianSearchCriteria. RandomDiscreteValueSearchCriteria sets max_runtime_secs to 0 by default which effectively means no time limit, but logic in ModelFeeder would behave unexpectedly here ( return false ):

{code:java}final boolean enoughTime = hyperspaceIterator.max_runtime_secs() > 0 && hyperspaceIterator.time_remaining_secs() > 0{code}

The problem is probably coming from two different approaches two specify unlimited search: Cartesian search uses Double.MAX_VALUE, Random search uses 0 ( i think this is the most recent and right approach).

There is another concern: we put this value to the job.start(... , ..., it.max_runtime_secs() ) in GridSearch(L156) and in case of unlimited search with max_runtime_secs=0 I'm not sure it is what is expected in start() method. Unfortunately I could not find javadocs for explanations.

Issue 2. By fixing Issue 1 one test in GridTest would start to fail. The original idea of the test is not clear to me but probably an assumption in the test should be corrected. Investigation led me to the existing inconsistency in GridSearch between javadoc and code. I have fixed the code to make it consistent with the comment. Let's discuss in PR.

exalate-issue-sync[bot] commented 1 year ago

Andrey Spiridonov commented: [~accountid:5b153fb1b0d76456f36daced] you probably should know better what for we are using third parameter of job.start method ( see description above)

exalate-issue-sync[bot] commented 1 year ago

Sebastien Poirier commented: {{max_runtime_secs}} argument in {{job.start}} is only indicative: used for progress bar on clients.

exalate-issue-sync[bot] commented 1 year ago

Andrey Spiridonov commented: [~accountid:5b153fb1b0d76456f36daced] this was my understanding as well. Just worried about cases when we pass there zeroes

exalate-issue-sync[bot] commented 1 year ago

Andrey Spiridonov commented: [~accountid:5a32df017dcf343865c26fa5] could you please take a look at this one? see corresponding PR for the fix

exalate-issue-sync[bot] commented 1 year ago

Michal Kurka commented: [~accountid:557058:6e44bc1a-dd50-499b-a331-2e049f28773b] should probably skip the release notes since the affected feature was not yet released

exalate-issue-sync[bot] commented 1 year ago

Angela Bartz commented: Confirming that this will not be in the release notes.

h2o-ops commented 1 year ago

JIRA Issue Migration Info

Jira Issue: PUBDEV-7121 Assignee: Andrey Spiridonov Reporter: Andrey Spiridonov State: Resolved Fix Version: 3.28.0.1 Attachments: N/A Development PRs: Available

Linked PRs from JIRA

https://github.com/h2oai/h2o-3/pull/4119