Remove param 'evals_result' 'early_stopping_rounds' in lightgbm version > 3.3.1#5206
Conversation
Signed-off-by: Liang Zhang <liang.zhang@databricks.com>
| for cb in kwargs["callbacks"]: | ||
| if ( | ||
| hasattr(cb, "__qualname__") | ||
| and cb.__qualname__ == "early_stopping.<locals>._callback" |
There was a problem hiding this comment.
This checking is a little tricky , is there better way ? I think you want to check whether the callback is lgb.early_stopping(5) , can we directly compare the function object with it ?
There was a problem hiding this comment.
Directly comparing function object seems not working:
lightgbm.early_stopping(stopping_rounds=1) is lightgbm.early_stopping(stopping_rounds=1) # False
lightgbm.early_stopping(stopping_rounds=1) == lightgbm.early_stopping(stopping_rounds=1) # False
WeichenXu123
left a comment
There was a problem hiding this comment.
LGTM except one comment
Signed-off-by: Liang Zhang <liang.zhang@databricks.com>
| early_stopping = ( | ||
| num_pos_args >= early_stopping_index + 1 or "early_stopping_rounds" in kwargs | ||
| ) | ||
| early_stopping = model.best_iteration > 0 |
There was a problem hiding this comment.
model.best_iteration > 0 can be used to determine whether early stopping is activated.
model.best_iteration is initiated as 0 and is assigned with a positive int when EarlyStoppingException is raised (this means early stopping is activated, including using the early_stopping_rounds param, lightgbm.early_stopping callback function, or even a user-defined early_stopping callback that raises EarlyStoppingException).
model.best_iteration is returned as 0 when early stopping is not activated.
Signed-off-by: Liang Zhang <liang.zhang@databricks.com>
| new_backend = SageMakerBackend() | ||
| sagemaker_backends[region] = new_backend | ||
| # Create a SageMaker backend for EC2 region: "us-west-2" | ||
| sagemaker_backends = {"us-west-2": SageMakerBackend()} |
There was a problem hiding this comment.
According to the description in getmoto/moto#4699:
all backends are now generated on-request, rather then on start-up.
So we need to create a SageMaker backend for "us-west-2", which is used in our test. Since "us-west-2" is hardcoded in our tests as the default AWS region, I also hardcoded it here.
With moto==2.3.0, the old implementation only returns one region "us-east-1", which is the default region used by moto.
There was a problem hiding this comment.
We can probably update the small-requirements.txt now to instead of excluding version 2.0.7, just force use of >=2.3.0.
There was a problem hiding this comment.
This change is also compatible with moto<2.3.0, so we don't have to change it now.
* Add method to load model input example Signed-off-by: Matthieu Maitre <mmaitre@microsoft.com> * Try fixing doc and lint failures Signed-off-by: Matthieu Maitre <mmaitre@microsoft.com> * Remove param 'evals_result' 'early_stopping_rounds' in lightgbm version > 3.3.1 (#5206) The parameter evals_result is removed in the master of lightgbm: lightgbm-org/LightGBM#4882. The parameter early_stopping_rounds is removed in the master of lightgbm: lightgbm-org/LightGBM#4908. We should also remove this param in our test. This PR also fixed the sagemaker test failure. Signed-off-by: Liang Zhang <liang.zhang@databricks.com> * Address PR feedback Signed-off-by: Matthieu Maitre <mmaitre@microsoft.com> Co-authored-by: Liang Zhang <liang.zhang@databricks.com>
Signed-off-by: Liang Zhang liang.zhang@databricks.com
What changes are proposed in this pull request?
The parameter
evals_resultis removed in the master of lightgbm: lightgbm-org/LightGBM#4882.The parameter
early_stopping_roundsis removed in the master of lightgbm: lightgbm-org/LightGBM#4908.We should also remove this param in our test.
This PR also fixed the sagemaker test failure.
How is this patch tested?
Existing tests.
Does this PR change the documentation?
ci/circleci: build_doccheck. If it's successful, proceed to thenext step, otherwise fix it.
Detailson the right to open the job page of CircleCI.Artifactstab.docs/build/html/index.html.Release Notes
Is this a user-facing change?
(Details in 1-2 sentences. You can just refer to another PR with a description if this PR is part of a larger change.)
What component(s), interfaces, languages, and integrations does this PR affect?
Components
area/artifacts: Artifact stores and artifact loggingarea/build: Build and test infrastructure for MLflowarea/docs: MLflow documentation pagesarea/examples: Example codearea/model-registry: Model Registry service, APIs, and the fluent client calls for Model Registryarea/models: MLmodel format, model serialization/deserialization, flavorsarea/projects: MLproject format, project running backendsarea/scoring: MLflow Model server, model deployment tools, Spark UDFsarea/server-infra: MLflow Tracking server backendarea/tracking: Tracking Service, tracking client APIs, autologgingInterface
area/uiux: Front-end, user experience, plotting, JavaScript, JavaScript dev serverarea/docker: Docker use across MLflow's components, such as MLflow Projects and MLflow Modelsarea/sqlalchemy: Use of SQLAlchemy in the Tracking Service or Model Registryarea/windows: Windows supportLanguage
language/r: R APIs and clientslanguage/java: Java APIs and clientslanguage/new: Proposals for new client languagesIntegrations
integrations/azure: Azure and Azure ML integrationsintegrations/sagemaker: SageMaker integrationsintegrations/databricks: Databricks integrationsHow should the PR be classified in the release notes? Choose one:
rn/breaking-change- The PR will be mentioned in the "Breaking Changes" sectionrn/none- No description will be included. The PR will be mentioned only by the PR number in the "Small Bugfixes and Documentation Updates" sectionrn/feature- A new user-facing feature worth mentioning in the release notesrn/bug-fix- A user-facing bug fix worth mentioning in the release notesrn/documentation- A user-facing documentation change worth mentioning in the release notes