Add disable=True/False flag for Spark autologging by mohamad-arabi · Pull Request #20 · dbczumar/mlflow

mohamad-arabi · 2020-12-11T19:21:33Z

What changes are proposed in this pull request?

(Please fill in changes proposed in this fix)

How is this patch tested?

(Details)

Release Notes

Is this a user-facing change?

No. You can skip the rest of this section.
Yes. Give a description of this change to be included in the release notes for MLflow users.

(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

Interface

area/uiux: Front-end, user experience, JavaScript, plotting
area/docker: Docker use across MLflow's components, such as MLflow Projects and MLflow Models
area/sqlalchemy: Use of SQLAlchemy in the Tracking Service or Model Registry
area/windows: Windows support

Language

language/r: R APIs and clients
language/java: Java APIs and clients
language/new: Proposals for new client languages

Integrations

integrations/azure: Azure and Azure ML integrations
integrations/sagemaker: SageMaker integrations
integrations/databricks: Databricks integrations

How should the PR be classified in the release notes? Choose one:

rn/breaking-change - The PR will be mentioned in the "Breaking Changes" section
rn/none - No description will be included. The PR will be mentioned only by the PR number in the "Small Bugfixes and Documentation Updates" section
rn/feature - A new user-facing feature worth mentioning in the release notes
rn/bug-fix - A user-facing bug fix worth mentioning in the release notes
rn/documentation - A user-facing documentation change worth mentioning in the release notes

Signed-off-by: Mohamad Arabi <mohamad.arabi@databricks.com>

Signed-off-by: harupy <17039389+harupy@users.noreply.github.com>

…ion tests (mlflow#3800) * skip if the matrix is empty Signed-off-by: harupy <17039389+harupy@users.noreply.github.com> * set is_matrix_empty Signed-off-by: harupy <17039389+harupy@users.noreply.github.com> * fix syntax error Signed-off-by: harupy <17039389+harupy@users.noreply.github.com> * minor comment fix Signed-off-by: harupy <17039389+harupy@users.noreply.github.com>

* Fix for xgboost 1.3.0 Signed-off-by: harupy <17039389+harupy@users.noreply.github.com> * do not include 1.3.0 since it has been removed Signed-off-by: harupy <17039389+harupy@users.noreply.github.com> * re-run all the tests if set_matrix contains changes Signed-off-by: harupy <17039389+harupy@users.noreply.github.com> * nit Signed-off-by: harupy <17039389+harupy@users.noreply.github.com> * fix regexp Signed-off-by: harupy <17039389+harupy@users.noreply.github.com> * add test case Signed-off-by: harupy <17039389+harupy@users.noreply.github.com> * Refactor using packaging Signed-off-by: harupy <17039389+harupy@users.noreply.github.com> * add packaging Signed-off-by: harupy <17039389+harupy@users.noreply.github.com> * nit Signed-off-by: harupy <17039389+harupy@users.noreply.github.com>

Signed-off-by: Mohamad Arabi <mohamad.arabi@databricks.com>

dbczumar · 2020-12-12T00:02:22Z

tests/spark_autologging/test_spark_disable_autologging.py

+from tests.spark_autologging.utils import file_path  # pylint: disable=unused-import
+
+
+# Note that the following tests run one-after-the-other and operate on the SAME spark_session


@mohamad-arabi This is awesome! Can we also test the case where there isn't a preexisting spark session and we call autolog() with disable=True/False before creating a session?

In my opinion that is covered in these 2 test cases:
https://github.com/dbczumar/mlflow/pull/20/files#diff-7e304d4fffb199878e90000a4dd020fb4d8b5b9a046289a55babd3e44314e643R113

and

https://github.com/mlflow/mlflow/blob/master/tests/spark_autologging/test_spark_datasource_autologging.py#L228

These just test that no exceptions are thrown. They don’t verify that tags are set or not set depending on the disable flag when a session is created after autolog is called. Let me know if you think we already capture that elsewhere

…3682) * Safe Signed-off-by: Corey Zumar <corey.zumar@databricks.com> * Keras Signed-off-by: Corey Zumar <corey.zumar@databricks.com> * Lint Signed-off-by: Corey Zumar <corey.zumar@databricks.com> * TF Signed-off-by: Corey Zumar <corey.zumar@databricks.com> * Fixes Signed-off-by: Corey Zumar <corey.zumar@databricks.com> * Lint Signed-off-by: Corey Zumar <corey.zumar@databricks.com> * Some unit tests Signed-off-by: Corey Zumar <corey.zumar@databricks.com> * More unit tests Signed-off-by: Corey Zumar <corey.zumar@databricks.com> * Test coverage for safe_patch Signed-off-by: Corey Zumar <corey.zumar@databricks.com> * Add public API for autologging integration configs Signed-off-by: Mohamad Arabi <mohamad.arabi@databricks.com> Signed-off-by: Corey Zumar <corey.zumar@databricks.com> * Remove big comment Signed-off-by: Corey Zumar <corey.zumar@databricks.com> * Conf tests Signed-off-by: Corey Zumar <corey.zumar@databricks.com> * Tests Signed-off-by: Corey Zumar <corey.zumar@databricks.com> * Mark large Signed-off-by: Corey Zumar <corey.zumar@databricks.com> * Whitespace Signed-off-by: Corey Zumar <corey.zumar@databricks.com> * Blackspace Signed-off-by: Corey Zumar <corey.zumar@databricks.com> * Rename Signed-off-by: Corey Zumar <corey.zumar@databricks.com> * Simplify, will raise integrations as separate PR Signed-off-by: Corey Zumar <corey.zumar@databricks.com> * Lint Signed-off-by: Corey Zumar <corey.zumar@databricks.com> * Black Signed-off-by: Corey Zumar <corey.zumar@databricks.com> * Remove test_mode_off for now Signed-off-by: Corey Zumar <corey.zumar@databricks.com> * Support positional arguments Signed-off-by: Corey Zumar <corey.zumar@databricks.com> * Docstring fix Signed-off-by: Corey Zumar <corey.zumar@databricks.com> * use match instead of comparison to str(exc) Signed-off-by: Corey Zumar <corey.zumar@databricks.com> * Black Signed-off-by: Corey Zumar <corey.zumar@databricks.com> * Forward args Signed-off-by: Corey Zumar <corey.zumar@databricks.com> * Lint Signed-off-by: Corey Zumar <corey.zumar@databricks.com> * Try importing mock from unittest? Signed-off-by: Corey Zumar <corey.zumar@databricks.com> * Fix import mock in statsmodel Signed-off-by: Corey Zumar <corey.zumar@databricks.com> * Revert "Fix import mock in statsmodel" This reverts commit a81e810. Signed-off-by: Corey Zumar <corey.zumar@databricks.com> * Black Signed-off-by: Corey Zumar <corey.zumar@databricks.com> * Support tuple Signed-off-by: Corey Zumar <corey.zumar@databricks.com> * Address more comments Signed-off-by: Corey Zumar <corey.zumar@databricks.com> * Stop patching log_param Signed-off-by: Corey Zumar <corey.zumar@databricks.com> Co-authored-by: Mohamad Arabi <mohamad.arabi@databricks.com>

* reject bool metric value Signed-off-by: Halil Coban <halil.coban@gmail.com> * add comment on why we check for bool Signed-off-by: Halil Coban <halil.coban@gmail.com>

* initial commit Signed-off-by: tomasatdatabricks <tomas.nykodym@databricks.com> * Updated tests to refelct new type conversions rules and to make sure we include hin message when necessary. Signed-off-by: tomasatdatabricks <tomas.nykodym@databricks.com> * fix tests. Signed-off-by: tomasatdatabricks <tomas.nykodym@databricks.com> * lint. Signed-off-by: tomasatdatabricks <tomas.nykodym@databricks.com> * fix. Signed-off-by: tomasatdatabricks <tomas.nykodym@databricks.com> * fix. Signed-off-by: tomasatdatabricks <tomas.nykodym@databricks.com> * fix. Signed-off-by: tomasatdatabricks <tomas.nykodym@databricks.com> * minor fix Signed-off-by: tomasatdatabricks <tomas.nykodym@databricks.com> * lint Signed-off-by: tomasatdatabricks <tomas.nykodym@databricks.com> * revert Signed-off-by: tomasatdatabricks <tomas.nykodym@databricks.com> * update Signed-off-by: tomasatdatabricks <tomas.nykodym@databricks.com> * Update doc. Signed-off-by: tomasatdatabricks <tomas.nykodym@databricks.com> * fix. Signed-off-by: tomasatdatabricks <tomas.nykodym@databricks.com> * fix docs. Signed-off-by: tomasatdatabricks <tomas.nykodym@databricks.com> * add hint/warning to schema inference Signed-off-by: tomasatdatabricks <tomas.nykodym@databricks.com> * Addressed review comments. Signed-off-by: tomasatdatabricks <tomas.nykodym@databricks.com> * Addressed review comments. Signed-off-by: tomasatdatabricks <tomas.nykodym@databricks.com>

…py < 3.0.0 (mlflow#3825) * Fix AttributeError: 'Dataset' object has no attribute 'value' Signed-off-by: harupy <17039389+harupy@users.noreply.github.com> * fix reimport Signed-off-by: harupy <17039389+harupy@users.noreply.github.com> * remove print Signed-off-by: harupy <17039389+harupy@users.noreply.github.com>

* Add gluon to cross-version-tests Signed-off-by: harupy <17039389+harupy@users.noreply.github.com> * fix version Signed-off-by: harupy <17039389+harupy@users.noreply.github.com> * Fix metric import Signed-off-by: harupy <17039389+harupy@users.noreply.github.com> * newline Signed-off-by: harupy <17039389+harupy@users.noreply.github.com> * fix typo & pylint error Signed-off-by: harupy <17039389+harupy@users.noreply.github.com> * use load_parameters Signed-off-by: harupy <17039389+harupy@users.noreply.github.com> * Fix import Signed-off-by: harupy <17039389+harupy@users.noreply.github.com> * Fix test_gluon_model_export.py Signed-off-by: harupy <17039389+harupy@users.noreply.github.com> * Add # pylint: disable=import-error Signed-off-by: harupy <17039389+harupy@users.noreply.github.com> * Fix import position Signed-off-by: harupy <17039389+harupy@users.noreply.github.com> * nit Signed-off-by: harupy <17039389+harupy@users.noreply.github.com>

* Fix invalid metric issue in statsmodels flavor Signed-off-by: harupy <17039389+harupy@users.noreply.github.com> * Introduce _is_numeric Signed-off-by: harupy <17039389+harupy@users.noreply.github.com>

* Add fastai to the cross version tests Signed-off-by: harupy <17039389+harupy@users.noreply.github.com> * add sklearn Signed-off-by: harupy <17039389+harupy@users.noreply.github.com>

…low#3815) * Safe Signed-off-by: Corey Zumar <corey.zumar@databricks.com> * Keras Signed-off-by: Corey Zumar <corey.zumar@databricks.com> * Lint Signed-off-by: Corey Zumar <corey.zumar@databricks.com> * TF Signed-off-by: Corey Zumar <corey.zumar@databricks.com> * Fixes Signed-off-by: Corey Zumar <corey.zumar@databricks.com> * Lint Signed-off-by: Corey Zumar <corey.zumar@databricks.com> * Some unit tests Signed-off-by: Corey Zumar <corey.zumar@databricks.com> * More unit tests Signed-off-by: Corey Zumar <corey.zumar@databricks.com> * Test coverage for safe_patch Signed-off-by: Corey Zumar <corey.zumar@databricks.com> * Add public API for autologging integration configs Signed-off-by: Mohamad Arabi <mohamad.arabi@databricks.com> Signed-off-by: Corey Zumar <corey.zumar@databricks.com> * Remove big comment Signed-off-by: Corey Zumar <corey.zumar@databricks.com> * Conf tests Signed-off-by: Corey Zumar <corey.zumar@databricks.com> * Tests Signed-off-by: Corey Zumar <corey.zumar@databricks.com> * Mark large Signed-off-by: Corey Zumar <corey.zumar@databricks.com> * Whitespace Signed-off-by: Corey Zumar <corey.zumar@databricks.com> * Blackspace Signed-off-by: Corey Zumar <corey.zumar@databricks.com> * Rename Signed-off-by: Corey Zumar <corey.zumar@databricks.com> * Simplify, will raise integrations as separate PR Signed-off-by: Corey Zumar <corey.zumar@databricks.com> * Remove partial tensorflow Signed-off-by: Corey Zumar <corey.zumar@databricks.com> * Lint Signed-off-by: Corey Zumar <corey.zumar@databricks.com> * Black Signed-off-by: Corey Zumar <corey.zumar@databricks.com> * Updates from utils Signed-off-by: Corey Zumar <corey.zumar@databricks.com> * Remove test_mode_off for now Signed-off-by: Corey Zumar <corey.zumar@databricks.com> * Support positional arguments Signed-off-by: Corey Zumar <corey.zumar@databricks.com> * Docstring fix Signed-off-by: Corey Zumar <corey.zumar@databricks.com> * use match instead of comparison to str(exc) Signed-off-by: Corey Zumar <corey.zumar@databricks.com> * Black Signed-off-by: Corey Zumar <corey.zumar@databricks.com> * Forward args Signed-off-by: Corey Zumar <corey.zumar@databricks.com> * Fixes from mlflow#3682 Signed-off-by: Corey Zumar <corey.zumar@databricks.com> * integration start Signed-off-by: Corey Zumar <corey.zumar@databricks.com> * Lint Signed-off-by: Corey Zumar <corey.zumar@databricks.com> * Try importing mock from unittest? Signed-off-by: Corey Zumar <corey.zumar@databricks.com> * Fix import mock in statsmodel Signed-off-by: Corey Zumar <corey.zumar@databricks.com> * Mock fix Signed-off-by: Corey Zumar <corey.zumar@databricks.com> * Revert "Fix import mock in statsmodel" This reverts commit a81e810. Signed-off-by: Corey Zumar <corey.zumar@databricks.com> * Black Signed-off-by: Corey Zumar <corey.zumar@databricks.com> * Support tuple Signed-off-by: Corey Zumar <corey.zumar@databricks.com> * Address more comments Signed-off-by: Corey Zumar <corey.zumar@databricks.com> * Stop patching log_param Signed-off-by: Corey Zumar <corey.zumar@databricks.com> * Modules Signed-off-by: Corey Zumar <corey.zumar@databricks.com> * Another test, enable test mode broadly Signed-off-by: Corey Zumar <corey.zumar@databricks.com> * Black Signed-off-by: Corey Zumar <corey.zumar@databricks.com> * Fix Signed-off-by: Corey Zumar <corey.zumar@databricks.com> * Move to fixture Signed-off-by: Corey Zumar <corey.zumar@databricks.com> * Docstring Signed-off-by: Corey Zumar <corey.zumar@databricks.com> * Use test mode for try_mlflow_log Signed-off-by: Corey Zumar <corey.zumar@databricks.com> * Test try_mlflow_log Signed-off-by: Corey Zumar <corey.zumar@databricks.com> * Docs Signed-off-by: Corey Zumar <corey.zumar@databricks.com> * Assert Signed-off-by: Corey Zumar <corey.zumar@databricks.com> * Try log keras Signed-off-by: Corey Zumar <corey.zumar@databricks.com> * Review comment, add init for tests Signed-off-by: Corey Zumar <corey.zumar@databricks.com> * Lint Signed-off-by: Corey Zumar <corey.zumar@databricks.com> * Actually commit the fixtures file... Signed-off-by: Corey Zumar <corey.zumar@databricks.com> * Test fixes, lint Signed-off-by: Corey Zumar <corey.zumar@databricks.com> * Fix, format Signed-off-by: Corey Zumar <corey.zumar@databricks.com> * Fix fast.ai Signed-off-by: Corey Zumar <corey.zumar@databricks.com> * Lintfix Signed-off-by: Corey Zumar <corey.zumar@databricks.com> * Lint Signed-off-by: Corey Zumar <corey.zumar@databricks.com> * Docstrings Signed-off-by: Corey Zumar <corey.zumar@databricks.com> * Address nit Signed-off-by: Corey Zumar <corey.zumar@databricks.com> * Lint Signed-off-by: Corey Zumar <corey.zumar@databricks.com> Co-authored-by: Mohamad Arabi <mohamad.arabi@databricks.com>

Signed-off-by: Mohamad Arabi <mohamad.arabi@databricks.com>

mohamad-arabi and others added 6 commits December 9, 2020 16:24

Add public API for autologging integration configs

bcd1ba3

Signed-off-by: Mohamad Arabi <mohamad.arabi@databricks.com>

Prefix with Lightning (mlflow#3809)

1c72efe

Signed-off-by: harupy <17039389+harupy@users.noreply.github.com>

Add support for disabling spark autologging

3ce637f

Signed-off-by: Mohamad Arabi <mohamad.arabi@databricks.com>

Merge branch 'interf_proto' into disable-flag-spark

b865cce

Signed-off-by: Mohamad Arabi <mohamad.arabi@databricks.com>

mohamad-arabi changed the base branch from master to interf_proto December 11, 2020 19:22

mohamad-arabi added 2 commits December 11, 2020 11:23

Remove unused method

26da1b4

Signed-off-by: Mohamad Arabi <mohamad.arabi@databricks.com>

Remove unused method part 2

b2d0771

Signed-off-by: Mohamad Arabi <mohamad.arabi@databricks.com>

dbczumar reviewed Dec 12, 2020

View reviewed changes

dbczumar and others added 13 commits December 11, 2020 17:17

reject bool metric value (mlflow#3822)

2826878

* reject bool metric value Signed-off-by: Halil Coban <halil.coban@gmail.com> * add comment on why we check for bool Signed-off-by: Halil Coban <halil.coban@gmail.com>

Fix invalid metric error in statsmodels tests (mlflow#3828)

26fed59

* Fix invalid metric issue in statsmodels flavor Signed-off-by: harupy <17039389+harupy@users.noreply.github.com> * Introduce _is_numeric Signed-off-by: harupy <17039389+harupy@users.noreply.github.com>

Add fastai to the cross version tests (mlflow#3830)

0be7352

* Add fastai to the cross version tests Signed-off-by: harupy <17039389+harupy@users.noreply.github.com> * add sklearn Signed-off-by: harupy <17039389+harupy@users.noreply.github.com>

add test case for before spark session

94bc305

Signed-off-by: Mohamad Arabi <mohamad.arabi@databricks.com>

Merge branch master into disable-flag-spark

c538fdf

Signed-off-by: Mohamad Arabi <mohamad.arabi@databricks.com>

unnecessary change

9eac976

Signed-off-by: Mohamad Arabi <mohamad.arabi@databricks.com>

modify comment

0ec7400

Signed-off-by: Mohamad Arabi <mohamad.arabi@databricks.com>

cannot assign FLAVOR_NAME in _spark_autolgging.py

4feeb11

Signed-off-by: Mohamad Arabi <mohamad.arabi@databricks.com>

dbczumar mentioned this pull request Dec 14, 2020

Add disable=True/False flag for Spark autologging mlflow/mlflow#3838

Merged

27 tasks

mohamad-arabi added 3 commits December 14, 2020 13:14

address final comments

cf7a1a0

Signed-off-by: Mohamad Arabi <mohamad.arabi@databricks.com>

fix api documentation

a708ada

Signed-off-by: Mohamad Arabi <mohamad.arabi@databricks.com>

fix api documentation II

bc59ab0

Signed-off-by: Mohamad Arabi <mohamad.arabi@databricks.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add disable=True/False flag for Spark autologging#20

Add disable=True/False flag for Spark autologging#20
mohamad-arabi wants to merge 24 commits intodbczumar:interf_protofrom
mohamad-arabi:disable-flag-spark

mohamad-arabi commented Dec 11, 2020

Uh oh!

dbczumar Dec 12, 2020

Uh oh!

mohamad-arabi Dec 12, 2020

Uh oh!

dbczumar Dec 12, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

		from tests.spark_autologging.utils import file_path # pylint: disable=unused-import


		# Note that the following tests run one-after-the-other and operate on the SAME spark_session

Conversation

mohamad-arabi commented Dec 11, 2020

What changes are proposed in this pull request?

How is this patch tested?

Release Notes

Is this a user-facing change?

What component(s), interfaces, languages, and integrations does this PR affect?

How should the PR be classified in the release notes? Choose one:

Uh oh!

dbczumar Dec 12, 2020

Choose a reason for hiding this comment

Uh oh!

mohamad-arabi Dec 12, 2020

Choose a reason for hiding this comment

Uh oh!

dbczumar Dec 12, 2020

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants