TensorFlow 1.13 support #56

yongtang · 2019-01-15T04:33:02Z

This PR tries to add TensorFlow 1.13 support. As 1.13 has not been released yet, this PR is a working in progress until the release of 1.13.

Since 1.13 have breaking changes for Dataset API, this PR can only be merged with 1.13 released.

This PR fixes #54.

yongtang · 2019-01-15T05:52:44Z

Looks like almost all tests passed. Though Travis CI's python 3.6 will crash when import tf-nightly.

We could wait until tf 1.13rc is out to take another look. We also could decide which version or platform we plan to support.

yongtang · 2019-01-15T22:28:24Z

When installing the generated whl files, the .so encountered an issue of:

_video_ops_libav_9.20.so: undefined symbol: _ZN10tensorflow14kernel_factory17OpKernelRegistrar12InitInternalEPKNS_9KernelDefEN4absl11string_viewESt10unique_ptrINS0_15OpKernelFactoryESt14default_deleteIS8_EE

Need some further investigation.

yongtang · 2019-01-16T02:15:59Z

Think I found where undefined symbol comes from. In setup.py the requirement is tensorflow, not tf-nightly. So even if we install tf-nightly it will still reinstall tensorflow. That will cause the api mismatch.

Change setup.py to use tf-nightly will fix the issue.

dmitrievanthony · 2019-01-21T08:37:02Z

tensorflow_io/hadoop/python/kernel_tests/hadoop_test.py

@@ -48,7 +49,7 @@ def test_sequence_file_dataset(self):

    dataset = hadoop_dataset_ops.SequenceFileDataset(filenames).repeat(
        num_repeats)
-    iterator = dataset.make_initializable_iterator()
+    iterator = dataset_ops.make_initializable_iterator(dataset)


I guess it would be better to use tf.compat.v1.data.make_one_shot_iterator(dataset) as it's mentioned in https://github.com/tensorflow/tensorflow/blob/master/tensorflow/python/data/ops/dataset_ops.py#L1358. What do you think?

Thanks @dmitrievanthony. Updated and using tensorflow.compat.v1.data now.

dmitrievanthony · 2019-01-21T08:39:43Z

tensorflow_io/ignite/python/tests/ignite_dataset_test.py

@@ -66,7 +67,7 @@ def _check_dataset(self, dataset):
    self.assertEqual(dtypes.string, dataset.output_types["val"]["NAME"])
    self.assertEqual(dtypes.int64, dataset.output_types["val"]["VAL"])

-    it = dataset.make_one_shot_iterator()
+    it = dataset_ops.make_one_shot_iterator(dataset)


Would be great to update README.md (in ignite module and in the root folder) as well. We can do it in the same PR (you can take this version as a reference) or I can do it in a separate PR.

BryanCutler · 2019-01-23T04:41:51Z

@yongtang I figured out the issue with the ArrowDatasets and will put up the fix soon

yongtang · 2019-01-28T18:33:18Z

I think we are almost complete for 1.13 support. Unfortunately R may need to be updated, but this could be done later.

Also this PR will not be merged until TensorFlow 1.13 is released (At the moment only 1.13rc0). I assume it will happen in several weeks or a month or two.

yongtang · 2019-01-28T18:33:44Z

Also cc @terrytangyuan @BryanCutler FYI.

yongtang · 2019-02-07T00:38:09Z

I am continuing to play with tf 1.12.0 vs. 1.13.0. Noticed that the C++ binaries build against 1.12.0 will not work with 1.13.0, as the following has changed:

# 1.12.0:
tensorflow::kernel_factory::OpKernelRegistrar::InitInternal( \
  tensorflow::KernelDef const*, absl::string_view, \
  tensorflow::OpKernel* (*)(tensorflow::OpKernelConstruction*))
# 1.13.0
tensorflow::kernel_factory::OpKernelRegistrar::InitInternal( \
  tensorflow::KernelDef const*, absl::string_view, \
  std::unique_ptr<tensorflow::kernel_factory::OpKernelFactory, \
  std::default_delete<tensorflow::kernel_factory::OpKernelFactory> >)

That means we do have to release a binary against 1.13.0 even if we use tensorflow's public endpoint Python API.

terrytangyuan · 2019-02-07T18:34:58Z

The class name has changed. We probably need to update this line: https://github.com/tensorflow/io/blob/master/R-package/R/dataset_utils.R#L10

Try change it to something like:

inherits(x, c("tensorflow.python.data.ops.dataset_ops.Dataset", "tensorflow.compat.v1.data.Dataset")) || is_tfio_dataset(x)

If this doesn't work, temporary solution is to have that is_dataset() function return TRUE to unblock this PR and I'll look into it later:

is_dataset <- function (x) {
  # inherits(x, "tensorflow.python.data.ops.dataset_ops.Dataset") || is_tfio_dataset(x)
  TRUE
}

BryanCutler

LGTM, just one minor comment

BryanCutler · 2019-02-07T23:15:15Z

tensorflow_io/arrow/python/ops/arrow_dataset_ops.py

+    self._output_types = output_types
+    self._output_shapes = output_shapes or \
+        nest.map_structure(
+            lambda _: tensorflow.TensorShape(None), self._output_types)


I could be wrong, but I don't think these need to be initialized here and could still be in the base class constructor, even though the call needs to be last?

@BryanCutler I just updated the PR and removed unneeded initialization (other than one place which is needed).

Thanks, looks good!

yongtang · 2019-02-07T23:17:42Z

Thanks @terrytangyuan. I added the follwoing

inherits(x, c("tensorflow.python.data.ops.dataset_ops.Dataset", "tensorflow.python.data.ops.dataset_ops.DatasetV1", "tensorflow.python.data.ops.dataset_ops.DatasetV2")) || is_tfio_dataset(x)

and verified locally that the above check passes. However, the program still throw out Provided dataset is not a TensorFlow Dataset error even if I delete them completely.

I would guess that is because tfdatasets also performs the check:
https://github.com/rstudio/tfdatasets/blob/2af1c0fb6753fc5c6178dd0981125ff100761fc0/R/utils.R#L70-L71

Since last tfdatasets was released several months ago, I think tfdatasets may need to be updated as well to fix the R issue?

BryanCutler · 2019-02-07T23:26:13Z

Thanks for looking into this @yongtang !

terrytangyuan · 2019-02-08T03:40:20Z

@yongtang I doubt that it's related to tfdatasets since we are using our own is_dataset(). If the fix doesn't work, feel free to have it return TRUE and track this in a separate issue. I'll help look into further later. As long as class(dataset) <- c("tf_dataset", class(dataset)) logic is happening in as_tf_dataset(), this should be safe. This function is internal and for package developer only so I don't expect that random objects will be passed into as_tf_dataset().

yongtang · 2019-02-08T03:43:49Z

@terrytangyuan I tried locally with TRUE and still it thrown out error. On the other hand, if I install tfdatasets from the GitHub (master branch HEAD):

R -e 'library("devtools"); install_github("rstudio/tfdatasets", ref="c6fc59b")'

Then it seems the tests are passing. Wondering if this helps.

terrytangyuan · 2019-02-08T04:07:04Z

@yongtang I see. Then it might be using the is_dataset() from tfdatasets. I’ll double check on this. In the meantime, let’s just use your fix for now. Thanks for looking into this!

dmitrievanthony

LGTM, only a comment regarding tensorflow version in REQUIRED_PACKAGES.

dmitrievanthony · 2019-02-08T08:09:24Z

setup.py

 REQUIRED_PACKAGES = [
-    'tensorflow == 1.12.0',
+    'tensorflow >= 1.13.0rc0',


Does it makes sense to add upper bound as well?

dmitrievanthony · 2019-02-08T12:04:25Z

BTW, @yongtang are you planning to port these changes into tensorflow/contrib?

I see that last changes I made to recover ignite and kafka functionality in tensorflow were cherry-picked into 1.13 and so that 1.13.0rc0 works well. Unfortunately, it looks like folks changed underlying API again and our code is broken in 1.13.0rc1. I could fix this issue ad-hock, but we could also cherry-pick changes from this request into tensorflow/contrib also. What will be the best chose, what do you think?

yongtang · 2019-02-08T20:51:32Z

@dmitrievanthony My understanding is that tensorflow 1.13 will likely be the last release before 2.0. And starting from tensorflow 2.0, the contrib directory will be deprecated and removed from tensorflow repo.

In that case, an ad-hoc fix on tensorflow/contrib to r1.13 branch directly (so that some ops are working fine) might be an easier path.

/cc @martinwicke @ewilderj to comment on the timeline of 1.13 and 2.0.

martinwicke · 2019-02-08T20:53:16Z

Sadly, it's very likely we'll have at least one more 1.x release before 2.0 actually lands (outside of pre-releases).

yongtang · 2019-02-17T23:47:48Z

Both TF 1.13.0rc2 and TF 2.0 (tf-nightly-2.0-preview) build with tensorflow-io succeed now. Think we are ready for 1.13 and 2.0.

Note:

This PR is on top of another PR Travis CI Enhancement #94
The code base is compatible with 1.13 and 2.0. However,
the binary built with TF 1.13.0rc2 is not compatible with TF 2.0 (C++ API issue?), see TensorFlow 2.0 support #55 (comment)

I think the plan will be to rebuild another version of tensorflow-io when 2.0 is released.

Signed-off-by: Yong Tang <yong.tang.github@outlook.com>

Signed-off-by: Yong Tang <yong.tang.github@outlook.com> Use GitHub version of tfdataset in R Signed-off-by: Yong Tang <yong.tang.github@outlook.com>

Signed-off-by: Yong Tang <yong.tang.github@outlook.com>

yongtang · 2019-02-20T16:49:14Z

Will merge this PR in for now. We want to start testing tf 1.13 now (as tf 1.13 release is imminent).

I will create a follow up PR to change the pip requirement to tensorflow>=1.13.0, < 1.14.0 once TF 1.13 is released.

TensorFlow 1.13 support

yongtang force-pushed the 1.13 branch from 7d3b134 to a6fca32 Compare January 16, 2019 01:13

yongtang force-pushed the 1.13 branch from a6fca32 to 74f7726 Compare January 19, 2019 20:09

This was referenced Jan 19, 2019

Add Apache Arrow dataset support #36

Merged

Travis CI build enhancements #58

Merged

dmitrievanthony reviewed Jan 21, 2019

View reviewed changes

yongtang force-pushed the 1.13 branch 5 times, most recently from 86e6d1f to 23e59ee Compare January 25, 2019 05:10

yongtang mentioned this pull request Jan 25, 2019

Use tensorflow == 1.12.0 in setup.py #65

Merged

yongtang force-pushed the 1.13 branch 7 times, most recently from a4a6d1f to 1af5cdb Compare January 28, 2019 15:36

yongtang force-pushed the 1.13 branch from 1af5cdb to daf7a5e Compare January 29, 2019 19:23

yongtang force-pushed the 1.13 branch 2 times, most recently from fe2e007 to 9ac274c Compare February 6, 2019 22:39

yongtang force-pushed the 1.13 branch 2 times, most recently from 7e448d9 to e9bb8bd Compare February 7, 2019 02:57

BryanCutler reviewed Feb 7, 2019

View reviewed changes

yongtang force-pushed the 1.13 branch from 21a7a5e to bdc0ab8 Compare February 7, 2019 23:55

dmitrievanthony approved these changes Feb 8, 2019

View reviewed changes

yongtang force-pushed the 1.13 branch from 470d48a to dbafc76 Compare February 8, 2019 18:01

yongtang force-pushed the 1.13 branch 5 times, most recently from 740733c to 6883ab0 Compare February 17, 2019 22:06

yongtang force-pushed the 1.13 branch from 6883ab0 to 1d96e6e Compare February 18, 2019 17:52

Update tensorflow to 1.13rc1

264e472

Signed-off-by: Yong Tang <yong.tang.github@outlook.com>

yongtang force-pushed the 1.13 branch from 1d96e6e to 10daf83 Compare February 19, 2019 00:04

yongtang added 3 commits February 19, 2019 00:35

Fix tensorflow 1.13 issues with ops

2b33d4d

Signed-off-by: Yong Tang <yong.tang.github@outlook.com>

R check fix

8e6ce11

Signed-off-by: Yong Tang <yong.tang.github@outlook.com> Use GitHub version of tfdataset in R Signed-off-by: Yong Tang <yong.tang.github@outlook.com>

Enable 2.0 preview on Travis CI

ad3d82d

Signed-off-by: Yong Tang <yong.tang.github@outlook.com>

yongtang force-pushed the 1.13 branch from 10daf83 to ad3d82d Compare February 19, 2019 00:35

yongtang merged commit 02d4cb0 into tensorflow:master Feb 20, 2019

yongtang deleted the 1.13 branch February 20, 2019 16:49

i-ony pushed a commit to i-ony/io that referenced this pull request Feb 8, 2021

Merge pull request tensorflow#56 from yongtang/1.13

92dd89a

TensorFlow 1.13 support

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

TensorFlow 1.13 support #56

TensorFlow 1.13 support #56

yongtang commented Jan 15, 2019

yongtang commented Jan 15, 2019

yongtang commented Jan 15, 2019

yongtang commented Jan 16, 2019

dmitrievanthony Jan 21, 2019

yongtang Jan 28, 2019

dmitrievanthony Jan 21, 2019 •

edited

Loading

BryanCutler commented Jan 23, 2019

yongtang commented Jan 28, 2019

yongtang commented Jan 28, 2019

yongtang commented Feb 7, 2019

terrytangyuan commented Feb 7, 2019 •

edited

Loading

BryanCutler left a comment

BryanCutler Feb 7, 2019

yongtang Feb 8, 2019

BryanCutler Feb 8, 2019

yongtang commented Feb 7, 2019

BryanCutler commented Feb 7, 2019

terrytangyuan commented Feb 8, 2019

yongtang commented Feb 8, 2019

terrytangyuan commented Feb 8, 2019 •

edited

Loading

dmitrievanthony left a comment

dmitrievanthony Feb 8, 2019

dmitrievanthony commented Feb 8, 2019

yongtang commented Feb 8, 2019

martinwicke commented Feb 8, 2019

yongtang commented Feb 17, 2019

yongtang commented Feb 20, 2019

TensorFlow 1.13 support #56

TensorFlow 1.13 support #56

Conversation

yongtang commented Jan 15, 2019

yongtang commented Jan 15, 2019

yongtang commented Jan 15, 2019

yongtang commented Jan 16, 2019

dmitrievanthony Jan 21, 2019

Choose a reason for hiding this comment

yongtang Jan 28, 2019

Choose a reason for hiding this comment

dmitrievanthony Jan 21, 2019 • edited Loading

Choose a reason for hiding this comment

BryanCutler commented Jan 23, 2019

yongtang commented Jan 28, 2019

yongtang commented Jan 28, 2019

yongtang commented Feb 7, 2019

terrytangyuan commented Feb 7, 2019 • edited Loading

BryanCutler left a comment

Choose a reason for hiding this comment

BryanCutler Feb 7, 2019

Choose a reason for hiding this comment

yongtang Feb 8, 2019

Choose a reason for hiding this comment

BryanCutler Feb 8, 2019

Choose a reason for hiding this comment

yongtang commented Feb 7, 2019

BryanCutler commented Feb 7, 2019

terrytangyuan commented Feb 8, 2019

yongtang commented Feb 8, 2019

terrytangyuan commented Feb 8, 2019 • edited Loading

dmitrievanthony left a comment

Choose a reason for hiding this comment

dmitrievanthony Feb 8, 2019

Choose a reason for hiding this comment

dmitrievanthony commented Feb 8, 2019

yongtang commented Feb 8, 2019

martinwicke commented Feb 8, 2019

yongtang commented Feb 17, 2019

yongtang commented Feb 20, 2019

dmitrievanthony Jan 21, 2019 •

edited

Loading

terrytangyuan commented Feb 7, 2019 •

edited

Loading

terrytangyuan commented Feb 8, 2019 •

edited

Loading