New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Implementing TensorFlow 2.0 Compatibility #1872
Conversation
… tests for functionality
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
BTW one comment I meant to add earlier but forgot...we ideally should have a test where we load a MLflow model persisted with 1.x using TF 2.0 (both in function form & as a pyfunc). I think it's fine if we add such a test in a follow-up PR, but seems important to have before we claim TF 2.0 compatibility, and we should make sure the follow-up work is tracked somewhere.
@@ -30,8 +30,8 @@ | |||
from tests.helper_functions import mock_s3_bucket # pylint: disable=unused-import | |||
|
|||
SavedModelInfo = collections.namedtuple( | |||
"SavedModelInfo", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
BTW, super small nit but in general it's best to avoid these sort of no-op whitespace changes if possible (did your editor make them or something?). Not a big deal, it just makes the git blame
harder to trace through
Co-Authored-By: Siddharth Murching <smurching@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some small comments but otherwise looks good! Before we merge, we should manually test backwards compatibility when loading models persisted with TF 1.x using TF 2.0
…test regarding MLflow Exception
Co-Authored-By: Siddharth Murching <smurching@gmail.com>
Co-Authored-By: Siddharth Murching <smurching@gmail.com>
…ut of dataframe in tf2_wrapper
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM thanks @juntai-zheng!
MLflow now supports logging & loading models using TensorFlow 2.0.
What changes are proposed in this pull request?
In order for MLflow to be compatible with the upcoming TensorFlow 2.0, several under the hood changes had to be made in the examples, MLflow code, and tests. The main difference is how we load back models - instead of loading back signature definitions in conjunction with
tf.graphs
andsessions,
TF 2.0 allows us to abstract those away and load back models as callable functions for inference.The main API change is in
mlflow/tensorflow.py
with the functionload_model(model_uri,
tf_sess).tf_sess
is now optional, since TF 2.0 deprecatedsessions
. The code will error out if the user's TF version is < 2.0 and no argument is provided. The code will throw a warning thattf_sess
is deprecated and ignore the input if the user's TF version is 2.0+.In addition,
load_model()
originally returned aSignatureDef
. Now, the function returns a function which can be fed data and returns inputs. See the docs for details.The code has been modified to check for the user's TensorFlow version and be compatible with both TF 1.X and TF 2.0 (given correct arguments to APIs).
New files have been created for TF 2.0 tests and examples, with the underlying example dataset being changed to TensorFlow's
iris_data
example. A couple of tests testing loading a model into atf.graph
have been removed due to TF 2.0 abstracting graphs away. The Travis bash file has been modified to install the proper TF versions for each suite of tests.StrictVersion has been changed to LooseVersion for version checks in order to allow for letters in version numbers (such as TensorFlow 2.0.0rc2) to be valid version numbers.
This PR does not address autologging, keras, or keras autologging - those will be in a future PR.
How is this patch tested?
I created a new suite of tests (
mlflow/tests/tensorflow/test_tensorflow2_model_export.py
), based on the original tests in order to test the changes. The changes tomlflow/tensorflow.py
work with both the original test on TF 1.12 and with the new tests on TF 2.0.0rc2.Release Notes
Is this a user-facing change?
MLflow now supports TensorFlow 2.0.
load_model()
no longer takes atf_sess
, but still does if you are using TensorFlow 1.X. The function also returns a callable graph in the form of atf.function
rather than aSignatureDef
now.What component(s) does this PR affect?
How 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