[TF - Fix] Fix imports from tensorflow.python.keras with tf.version >= 2.5.0 #3157

DEKHTIARJonathan · 2021-09-08T00:17:04Z

Since Tensorflow 2.5, any import to tensorflow.python.keras is to be considered unstable / unsupported.

As a matter of fact, if you check the wheel manifest, starting with 2.5.0, Tensorflow directly depends on Keras:

Requires-Dist: numpy (~=1.19.2)
Requires-Dist: absl-py (~=0.10)
Requires-Dist: astunparse (~=1.6.3)
Requires-Dist: flatbuffers (~=1.12.0)
Requires-Dist: google-pasta (~=0.2)
Requires-Dist: h5py (~=3.1.0)
Requires-Dist: keras-preprocessing (~=1.1.2)
Requires-Dist: opt-einsum (~=3.3.0)
Requires-Dist: protobuf (>=3.9.2)
Requires-Dist: six (~=1.15.0)
Requires-Dist: termcolor (~=1.1.0)
Requires-Dist: typing-extensions (~=3.7.4)
Requires-Dist: wheel (~=0.35)
Requires-Dist: wrapt (~=1.12.1)
Requires-Dist: gast (==0.4.0)
Requires-Dist: tensorboard (~=2.5)
Requires-Dist: tensorflow-estimator (<2.6.0,>=2.5.0rc0)
Requires-Dist: keras-nightly (~=2.5.0.dev)                          # <============== Here
Requires-Dist: grpcio (~=1.34.0)

Due to the backward compatibility nature of HVD, we can't just remove the code. Hence, I am introducing conditional imports based on the value of tf.__version__

tgaddair

LGTM. The only question I have is how this relates to horovod.keras vs horovod.tensorflow.keras. We created the latter because TF moved Keras into TF natively, but now they're moving it back to its own repo. Should users just use horovod.keras going forward?

github-actions · 2021-09-08T01:48:36Z

Unit Test Results

  385 files ±0   385 suites ±0 5h 22m 0s ⏱️ + 8m 9s
  705 tests ±0   564 ✔️ -   2   139 💤 ±  0 2 ❌ +2
8 474 runs ±0 5 801 ✔️ +63 2 671 💤 - 65 2 ❌ +2

For more details on these failures, see this check.

Results for commit 4660683. ± Comparison against base commit 9cd8f79.

♻️ This comment has been updated with latest results.

DEKHTIARJonathan · 2021-09-08T17:51:31Z

@tgaddair : The only question I have is how this relates to horovod.keras vs horovod.tensorflow.keras. We created the latter because TF moved Keras into TF natively, but now they're moving it back to its own repo. Should users just use horovod.keras going forward?

IMHO if the "APIs" we depend on can exclusively use "import keras", then we should unify them going forward. Though code backward compat may not be straight forward. If we do this, let's do this in a subsequent PR.

DEKHTIARJonathan · 2021-09-08T19:14:11Z

@tgaddair let me give a bit of clarification, there used to be 3 ways to import Keras:

import keras
import tensorflow.keras
import tensorflow.python.keras # Should not be used anymore

Solution 3) is 100% deprecated and will be removed in TF 2.7 or 2.8.
Solution 2) is actually identical to 1) since TF 2.6.0: tensorflow/tensorflow@2e6acf753381

So knowing this, I think we should stop doing from tensorflow import keras and just go for import keras.
I'm going to edit my PR in this direction, and we can have a subsequent PR to unify horovod.keras and horovod.tensorflow.keras.

Does it sound fair to you ?

DEKHTIARJonathan · 2021-09-08T22:11:51Z

I fixed my PR everywhere except for Spark stuff. This part of the code is very convoluted, with "normal Keras" and "TF Keras". Which essentially now are the same.

Horovod now dynamically decides what import keras means depending of context:

if you access horovod.tensorflow => keras == keras (TF >= 2.6) or tensorflow.keras (TF < 2.6)
https://github.com/horovod/horovod/pull/3157/files#diff-bcd0a2a62875db179f5bff09c62dc2d49a9a1fa2549bfcd89c7c6fb9ec5302f5
if you access horovod.keras => keras == keras
https://github.com/horovod/horovod/pull/3157/files#diff-89ac9181848c99aa979d3c18fb43432d85b260fffa8f1092cae2736c9f1b83d2

chongxiaoc · 2021-09-24T21:33:36Z

I fixed my PR everywhere except for Spark stuff. This part of the code is very convoluted, with "normal Keras" and "TF Keras". Which essentially now are the same.

I can take a look of Spark estimator stuff.

chongxiaoc · 2021-09-27T05:22:26Z

After using this diff, keras module's load_model function is broken as below:

Sun Sep 26 22:19:57 2021[0]<stderr>:  File "/Users/chongxiaoc/build/hvd-py3.7/lib/python3.7/site-packages/horovod-0.22.1-py3.7-macosx-11-x86_64.egg/horovod/spark/task/__init__.py", line 61, in task_exec
Sun Sep 26 22:19:57 2021[0]<stderr>:    result = fn(*args, **kwargs)
Sun Sep 26 22:19:57 2021[0]<stderr>:  File "/Users/chongxiaoc/build/hvd-py3.7/lib/python3.7/site-packages/horovod-0.22.1-py3.7-macosx-11-x86_64.egg/horovod/spark/keras/remote.py", line 123, in train
Sun Sep 26 22:19:57 2021[0]<stderr>:    serialized_model, lambda x: hvd.load_model(x))
Sun Sep 26 22:19:57 2021[0]<stderr>:  File "/Users/chongxiaoc/build/hvd-py3.7/lib/python3.7/site-packages/horovod-0.22.1-py3.7-macosx-11-x86_64.egg/horovod/spark/keras/remote.py", line 284, in deserialize_keras_model
Sun Sep 26 22:19:57 2021[0]<stderr>:    return load_model_fn(f)
Sun Sep 26 22:19:57 2021[0]<stderr>:  File "/Users/chongxiaoc/build/hvd-py3.7/lib/python3.7/site-packages/horovod-0.22.1-py3.7-macosx-11-x86_64.egg/horovod/spark/keras/remote.py", line 123, in <lambda>
Sun Sep 26 22:19:57 2021[0]<stderr>:    serialized_model, lambda x: hvd.load_model(x))
Sun Sep 26 22:19:57 2021[0]<stderr>:  File "/Users/chongxiaoc/build/hvd-py3.7/lib/python3.7/site-packages/horovod-0.22.1-py3.7-macosx-11-x86_64.egg/horovod/tensorflow/keras/__init__.py", line 249, in load_model
Sun Sep 26 22:19:57 2021[0]<stderr>:    return _impl.load_model(keras, wrap_optimizer, _OPTIMIZER_MODULES, filepath, custom_optimizers, custom_objects)
Sun Sep 26 22:19:57 2021[0]<stderr>:  File "/Users/chongxiaoc/build/hvd-py3.7/lib/python3.7/site-packages/horovod-0.22.1-py3.7-macosx-11-x86_64.egg/horovod/_keras/__init__.py", line 207, in load_model
Sun Sep 26 22:19:57 2021[0]<stderr>:    return keras.models.load_model(filepath, custom_objects=horovod_objects)
Sun Sep 26 22:19:57 2021[0]<stderr>:  File "/Users/chongxiaoc/build/hvd-py3.7/lib/python3.7/site-packages/keras/models.py", line 225, in load_model
Sun Sep 26 22:19:57 2021[0]<stderr>:    f = h5py.File(filepath, mode='r')
Sun Sep 26 22:19:57 2021[0]<stderr>:  File "/Users/chongxiaoc/build/hvd-py3.7/lib/python3.7/site-packages/h5py/_hl/files.py", line 395, in __init__
Sun Sep 26 22:19:57 2021[0]<stderr>:    name = filename_encode(name)
Sun Sep 26 22:19:57 2021[0]<stderr>:  File "/Users/chongxiaoc/build/hvd-py3.7/lib/python3.7/site-packages/h5py/_hl/compat.py", line 111, in filename_encode
Sun Sep 26 22:19:57 2021[0]<stderr>:    filename = fspath(filename)
Sun Sep 26 22:19:57 2021[0]<stderr>:TypeError: expected str, bytes or os.PathLike object, not File

I cannot run keras_spark_mnist.py successfully, though the backed is pointed to tensorflow.keras, which is correct as expected.

chongxiaoc · 2021-10-04T00:48:42Z

Test in #3183:

Remove support of bare Keras in Spark estimator
Replace all import keras with import tensorflow.keras as keras

github-actions · 2021-10-22T22:59:43Z

Unit Test Results (with flaky tests)

  393 files +    4   393 suites +4 5h 38m 48s ⏱️ + 8m 11s
  705 tests ±    0   564 ✔️ -     2   139 💤 ±  0 2 ❌ +2
8 702 runs +114 6 003 ✔️ +172 2 693 💤 - 64 6 ❌ +6

For more details on these failures, see this check.

Results for commit 4660683. ± Comparison against base commit 9cd8f79.

♻️ This comment has been updated with latest results.

chongxiaoc · 2021-10-26T04:44:48Z

It looks like the version switching point should be tf 2.6.0.

I saw test_spark_keras.py::SparkKerasTests::test_model_serialization serialization failure for TF2, I will create a separate ticket for it.

stale · 2021-12-25T07:39:01Z

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

EnricoMi · 2022-01-02T15:31:55Z

What is the reason this did not make it into master? Is this still a desirable fix?

chongxiaoc · 2022-01-03T18:00:45Z

I think TF 2.6.0 is still having failures. @EnricoMi

chongxiaoc · 2022-02-07T22:34:22Z

Continue and test in #3403

DEKHTIARJonathan requested a review from tgaddair September 8, 2021 00:18

DEKHTIARJonathan force-pushed the tensorflow_keras_fix branch from 8c43ff9 to d4585d9 Compare September 8, 2021 00:19

tgaddair reviewed Sep 8, 2021

View reviewed changes

tgaddair approved these changes Sep 8, 2021

View reviewed changes

DEKHTIARJonathan force-pushed the tensorflow_keras_fix branch from d4585d9 to 4b4b747 Compare September 8, 2021 22:11

DEKHTIARJonathan force-pushed the tensorflow_keras_fix branch 3 times, most recently from f3fdcad to b2b8d3c Compare September 8, 2021 22:38

chongxiaoc mentioned this pull request Sep 28, 2021

Fixed spark keras "IsADirectoryError: [Errno 21] Is a directory: '/tmp/tmpn_eenlao/checkpoint'" error #3099

Merged

4 tasks

chongxiaoc mentioned this pull request Oct 6, 2021

Bump version to 0.23.0 #3200

Merged

Fix Keras Import to load upstream Keras

af99210

DEKHTIARJonathan force-pushed the tensorflow_keras_fix branch from b2b8d3c to af99210 Compare October 22, 2021 21:27

set tf2.6.0 as a switching point between keras and tensorflow.keras

4660683

stale bot added the wontfix label Dec 25, 2021

stale bot closed this Jan 1, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[TF - Fix] Fix imports from tensorflow.python.keras with tf.version >= 2.5.0 #3157

[TF - Fix] Fix imports from tensorflow.python.keras with tf.version >= 2.5.0 #3157

DEKHTIARJonathan commented Sep 8, 2021 •

edited

tgaddair left a comment

github-actions bot commented Sep 8, 2021 •

edited

DEKHTIARJonathan commented Sep 8, 2021

DEKHTIARJonathan commented Sep 8, 2021 •

edited

DEKHTIARJonathan commented Sep 8, 2021 •

edited

chongxiaoc commented Sep 24, 2021

chongxiaoc commented Sep 27, 2021

chongxiaoc commented Oct 4, 2021

github-actions bot commented Oct 22, 2021 •

edited

chongxiaoc commented Oct 26, 2021

stale bot commented Dec 25, 2021

EnricoMi commented Jan 2, 2022

chongxiaoc commented Jan 3, 2022

chongxiaoc commented Feb 7, 2022

[TF - Fix] Fix imports from tensorflow.python.keras with tf.__version__ >= 2.5.0 #3157

[TF - Fix] Fix imports from tensorflow.python.keras with tf.__version__ >= 2.5.0 #3157

Conversation

DEKHTIARJonathan commented Sep 8, 2021 • edited

tgaddair left a comment

Choose a reason for hiding this comment

github-actions bot commented Sep 8, 2021 • edited

Unit Test Results

DEKHTIARJonathan commented Sep 8, 2021

DEKHTIARJonathan commented Sep 8, 2021 • edited

DEKHTIARJonathan commented Sep 8, 2021 • edited

chongxiaoc commented Sep 24, 2021

chongxiaoc commented Sep 27, 2021

chongxiaoc commented Oct 4, 2021

github-actions bot commented Oct 22, 2021 • edited

Unit Test Results (with flaky tests)

chongxiaoc commented Oct 26, 2021

stale bot commented Dec 25, 2021

EnricoMi commented Jan 2, 2022

chongxiaoc commented Jan 3, 2022

chongxiaoc commented Feb 7, 2022

[TF - Fix] Fix imports from tensorflow.python.keras with tf.version >= 2.5.0 #3157

[TF - Fix] Fix imports from tensorflow.python.keras with tf.version >= 2.5.0 #3157

DEKHTIARJonathan commented Sep 8, 2021 •

edited

github-actions bot commented Sep 8, 2021 •

edited

DEKHTIARJonathan commented Sep 8, 2021 •

edited

DEKHTIARJonathan commented Sep 8, 2021 •

edited

github-actions bot commented Oct 22, 2021 •

edited