[OpenCL] Implementation improvements #9117

lukeiwanski · 2017-04-10T22:18:54Z

OpenCL implementation improvements (#22)

[Build] Use gcc/g++ as a host compiler to avoid Protobufs are into multiple shared libraries loaded from python #8394 (Support cuda 7.5 and cudnn 7.0 #54)
[Eigen] Version bump
[OpenCL] Implementation improvements
- Register SYCL implementations for random ops
  - Simplify by using Eigen math functions
- Registers Scatter and ScatterNd Ops for SYCL
- Registers Stack op for SYCL
- Fixes No sycl buffer found error for debug ops
- Registers MatMul and Transpose Ops to SYCL device for double
- Extends analyzer_cli_test.py test to cover SYCL
- Fixes Transpose Op for double when on SYCL
- Bumps Eigen version to fix double precision issue on SYCL
- Extends SessionDebugTestBase to cover SYCL
- Bumps Eigen Version
- Refactors Ops registration
- Introduces workaround for Const Op related to the difference between
  CUDA which uses pointers and OpenCL that uses buffers/accessors
- Extends memory types to cover DEVICE_SYCL as well
- Introduces GetSYCLDevice() method that returns list of supported devices
  with GPU device having the highest priority ( doesn't include blacklisted devices )
- ::internal::Transpose -> tensorflow::internal::Transpose in order to
  avoid compilation reported error
- Adds sycl_runtime to bazels ARRAY_DEPS
- Replicates TF_CALL_GPU_PROXY_TYPES for SYCL
- Fixes an issue caused by switch to aligned allocator for sycl buffer (Can't install on ubuntu 12.04.5 LTS #53)
- Fix testSimple and testConst in stack_op_test (JVM, .NET Language Support #3)
- RandomGamma has no GPU friendly implementation (virtualenv python 2.7.6 import tensorflow. TypeError: __init__() got an unexpected keyword argument 'syntax' #57)
- Register batch normalization kernels for OpenCL (ImportError: No module named core.framework.graph_pb2 #61)
- Compatibility fixes for TensorFlow 1.1.0-rc1
- Fixes Scatter Op
- Implements BatchMatmul Op for SYCL
- Lowercase the device name when GPU or SYCL returned
- kernel_estimator_test.py assertEqual-> assertAlmostEqual due to floating point
  representation on the device

tensorflow-jenkins · 2017-04-10T22:18:56Z

Can one of the admins verify this patch?

googlebot · 2017-04-10T22:18:58Z

So there's good news and bad news.

👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there.

😕 The bad news is that it appears that one or more commits were authored by someone other than the pull request submitter. We need to confirm that they're okay with their commits being contributed to this project. Please have them confirm that here in the pull request.

Note to project maintainer: This is a terminal state, meaning the cla/google commit status will not change from this state. It's up to you to confirm consent of the commit author(s) and merge this pull request when appropriate.

lukeiwanski · 2017-04-10T22:21:07Z

@ville-k, @Zakor94 can you confirm?

drpngx · 2017-04-10T23:18:42Z

Sometimes googlebot gets confused, could you git pull --rebase and push again?

* Avoid functions that might not be defined on SYCL device * Simplify by using Eigen math functions

* Registers Scatter and ScatterNd Ops for SYCL * Registers Stack op for SYCL * Fixes No sycl buffer found error for debug ops * Registers MatMul and Transpose Ops to SYCL device for double * Extends analyzer_cli_test.py test to cover SYCL * Fixes Transpose Op for double when on SYCL * Bumps Eigen version to fix double precision issue on SYCL * Extends SessionDebugTestBase to cover SYCL

- Bumps Eigen Version - Refactors Ops registration - Introduces workaround for Const Op related to the difference between CUDA which uses pointers and OpenCL that uses buffers/accessors - Extends memory types to cover DEVICE_SYCL as well - Introduces GetSYCLDevice() method that returns list of supported devices with GPU device having the highest priority ( doesn't include blacklisted devices ) - ::internal::Transpose -> tensorflow::internal::Transpose in order to avoid compilation reported error - re-introduces fix for bugged string replacement causing a lot of compilation warnings -c -> --include - Adds sycl_runtime to bazels ARRAY_DEPS - Replicates TF_CALL_GPU_PROXY_TYPES for SYCL

…l buffer (#53)

* Fix testSimple and testConst in stack_op_test * Create a specialisation of DoParallelConcatUpdate for SyclDevice and register it * Guard all code in TENSORFLOW_USE_SYCL * Do not use sycl device for int32 * Registration of the Sycl version is now looking like the one for the GPU * Remove added empty line

… to floating point representation on the device

ville-k · 2017-04-11T05:59:37Z

@lukeiwanski @googlebot ok with me.

Zakor94 · 2017-04-11T06:17:29Z

@lukeiwanski @googlebot yes ok with me too.

tensorflow-jenkins · 2017-05-11T21:15:47Z

Can one of the admins verify this patch?

rmlarsen · 2017-05-16T15:27:46Z

@benoitsteiner could you please take another look?

Fixes bazels missing undeclared inclusions that appeared after merge with TensorFlow upstream

rmlarsen · 2017-05-16T23:00:19Z

@tensorflow-jenkins test this please

lukeiwanski · 2017-05-17T15:17:23Z

The MacOs CPU Tests seems to be failing due to a missing dependency in gif_archive..

ERROR: /private/var/tmp/_bazel_jenkins/13e370a18c169b19baeafefb05212b85/external/gif_archive/BUILD.bazel:8:1: undeclared inclusion(s) in rule '@gif_archive//:gif':
this rule is missing dependency declarations for the following files included by 'external/gif_archive/lib/openbsd-reallocarray.c':

Is that caused by our changeset or is it present in the upstream? I cannot see how we could cause it.

rmlarsen · 2017-05-17T16:33:30Z

@lukeiwanski no, the build error on MacOS is unrelated to your changes.

gunan · 2017-05-26T20:53:23Z

configure

@@ -683,7 +683,7 @@ if [ "$TF_NEED_OPENCL" == "1" ]; then
 while true; do
  fromuser=""
  if [ -z "$HOST_CXX_COMPILER" ]; then
-    default_cxx_host_compiler=$(which clang++-3.6 || true)
+    default_cxx_host_compiler=$(which g++-4.8 || true)


Is there a reason to pin this to a specific gcc/g++ release?
Why not just "which g++"?
This can make configuration more difficult on different machines

We locked to GCC-4.8 as it was default installation on Ubuntu 14.04.
This has been changed in 50197b9 and d777320
However, on ubuntu 16.04 user will have to add "-D_GLIBCXX_USE_CXX11_ABI=0" manually to the command line if there is custom compiler installed on the machine.

gunan · 2017-05-26T20:53:31Z

configure

@@ -707,7 +707,7 @@ done
 while true; do
  fromuser=""
  if [ -z "$HOST_C_COMPILER" ]; then
-    default_c_host_compiler=$(which clang-3.6 || true)
+    default_c_host_compiler=$(which gcc-4.8 || true)


ditto above

gunan · 2017-05-26T21:06:17Z

tensorflow/contrib/kernel_methods/python/kernel_estimators_test.py

@@ -92,7 +92,7 @@ def testLinearlySeparableBinaryDataNoKernels(self):
    # Since the data is linearly separable, the classifier should have small
    # loss and perfect accuracy.
    self.assertLess(metrics['loss'], 0.1)
-    self.assertEqual(metrics['accuracy'], 1.0)
+    self.assertAlmostEqual(metrics['accuracy'], 1.0, 7)


According to the comment above, I am worried about almost equal here. I know 7 digits is very very precise, but still we are making the test looser.

The reason for that was the fact that sometimes OpenCL represent this value as 0.99999996
Reverting in 1a60452

gunan · 2017-05-26T21:19:36Z

Jenkins, test this please.

…iler setup for Ubuntu 16.04

…qual due to floating point representation on the device" This reverts commit 06c50c0.

benoitsteiner · 2017-05-31T04:29:03Z

Jenkins, test this please.

* OpenCL Improvements * Registers Scatter and ScatterNd Ops for SYCL * Registers Stack op for SYCL * Fixes No sycl buffer found error for debug ops * Registers MatMul and Transpose Ops to SYCL device for double * Extends analyzer_cli_test.py test to cover SYCL * Fixes Transpose Op for double when on SYCL * Bumps Eigen version to fix double precision issue on SYCL * Extends SessionDebugTestBase to cover SYCL * Register SYCL implementations for random ops * Avoid functions that might not be defined on SYCL device (#51) * Avoid functions that might not be defined on SYCL device * Simplify by using Eigen math functions * OpenCL improvements - Bumps Eigen Version - Refactors Ops registration - Introduces workaround for Const Op related to the difference between CUDA which uses pointers and OpenCL that uses buffers/accessors - Extends memory types to cover DEVICE_SYCL as well - Introduces GetSYCLDevice() method that returns list of supported devices with GPU device having the highest priority ( doesn't include blacklisted devices ) - ::internal::Transpose -> tensorflow::internal::Transpose in order to avoid compilation reported error - re-introduces fix for bugged string replacement causing a lot of compilation warnings -c -> --include - Adds sycl_runtime to bazels ARRAY_DEPS - Replicates TF_CALL_GPU_PROXY_TYPES for SYCL * [OpenCL] Fixes an issue caused by switch to aligned allocator for sycl buffer (#53) * [Build] Use gcc/g++ as a host compiler to avoid tensorflow/tensorflow#8394 (#54) * [OpenCL] Fixes Scatter Op * Fix testSimple and testConst in stack_op_test (#3) * Fix testSimple and testConst in stack_op_test * Create a specialisation of DoParallelConcatUpdate for SyclDevice and register it * Guard all code in TENSORFLOW_USE_SYCL * Do not use sycl device for int32 * Registration of the Sycl version is now looking like the one for the GPU * Remove added empty line * Register batch normalization kernels for OpenCL (#61) * [OpenCL] RandomGamma has no GPU friendly implementation (#57) * [OpenCL] Compatibility fixes for TensorFlow 1.1.0-rc1 * [OpenCL] Implements BatchMatmul Op for SYCL * Lowercase the device name when GPU or SYCL returned * [OpenCL] kernel_estimator_test.py assertEqual-> assertAlmostEqual due to floating point representation on the device * [Eigen] Version bump * GPU device name string manipulation is not needed anymore * [OpenCL] Adds SYCL to device backwards compatibility * [OpenCL] Extends core_rnn_test.py to run for SYCL device * [OpenCL] Minor optimizations for build script * [OpenCL] Enables skip folder list in build script * [OpenCL] Fixes ApplyAdamOp for Sycl device * [OpenCL] SYCL device improvements * [OpenCL] Fixes debug_ops's SEGFAULT for SYCL device * [Build] Adds hexagon to skipped folders list * [OpenCL] Removes EnterLameDuckMode from SYCL device and allocator * [OpenCL] Registers Unique Op for SYCL device * [OpenCL][Temporary] Disables tests for SYCL target due to features not being implemented yet Tests affected: - tensorflow/contrib/memory_stats/python/kernel_tests/memory_stats_ops_test.py - tensorflow/contrib/rnn/python/kernel_tests/core_rnn_test.py - tensorflow/python/kernel_tests/conv_ops_test.py - tensorflow/python/kernel_tests/depthwise_conv_op_test.py - tensorflow/python/kernel_tests/pooling_ops_3d_test.py - tensorflow/python/kernel_tests/pooling_ops_test.py - tensorflow/python/kernel_tests/scatter_nd_ops_test.py - tensorflow/python/training/adam_test.py - tensorflow/python/training/localhost_cluster_performance_test.py - tensorflow/python/training/training_ops_test.py * [OpenCL][Temporary] Disables failing tests for SYCL in order to establish regression baseline Tests affected: - tensorflow/python/debug/cli/analyzer_cli_test.py - tensorflow/python/debug/lib/session_debug_testlib.py - tensorflow/python/debug/lib/stepper_test.py - tensorflow/python/kernel_tests/unstack_op_test.py - tensorflow/python/ops/image_ops_test.py * [OpenCL] Take options.config.device_count() into consideration * [OpenCL] Fixes compilation warning * [OpenCL] device:SYCL:0 -> sycl:0 * [OpenCL] Removes unwanted flags in building script Removes flags given to computecpp that enable SIMD instructions Removes duplicate flags * bool -> const bool * [OpenCL] sycl in test_util.gpu_device_name() -> is_sycl_enabled() * [OpenCL][Temporary] Disables failing tests for SYCL in order to establish regression baseline Test affected: - tensorflow/contrib/stateless/python/kernel_tests/stateless_random_ops_test.py * Imports test_util from tensorflow.python.framework * [OpenCL] Fixes formatting in Python code * [OpenCL] Extends session_test.py to cover SYCL device * [OpenCL] Cleans singleton class * [OpenCL] Keeping CUDA happy * [OpenCL][Temporary] Disables failing tests for SYCL in order to establish regression baseline Test affected: - tensorflow/contrib/rnn/python/kernel_tests/core_rnn_cell_test.py - tensorflow/contrib/seq2seq/python/kernel_tests/beam_search_ops_test.py * Added support for building with SYCL on ARM. * Acts on the review feedback from: - tensorflow/tensorflow#9117 (comment) - tensorflow/tensorflow#9117 (comment) * [OpenCL] Fixes scatter_nd_op_test * Fixes auto-merge mistake * [OpenCL] struct SyclDevice -> class SyclDevice * Revert "[OpenCL] struct SyclDevice -> class SyclDevice" This reverts commit addd43348c374a5379f67bb1e5ad084715722fc2. * [OpenCL] Reverting refactoring commit. As requested in the review tensorflow/tensorflow#9117 (comment) This change set will be re-introduced in smaller chunks. * Revert "[OpenCL] device:SYCL:0 -> sycl:0" This reverts commit cf16e60340b62d16c3764d71b716fe03d35f87a9. * Revert "[OpenCL] Adds SYCL to device backwards compatibility" This reverts commit b8401b5164199b7a169be1c1d8dea5001195c390. * Acts on the feedback from tensorflow/tensorflow#9117 (comment) * control_flow_ops_py_test.py expects device name to be lower cased * Acts on the feedback from tensorflow/tensorflow#9117 (comment) * Removes debug print * Removes not needed partial specialisation * [OpenCL] Registers ScatterNdFunctor for SYCL device * [OpenCL] Make it compile * [OpenCL] Follow gpu_device changes * [OpenCL] Adds cxx_builtin_include_directory for python lib Fixes bazels missing undeclared inclusions that appeared after merge with TensorFlow upstream * [OpenCL] Fixes Constant Op * [OpenCL] gXX-4.8 -> gXX * [OpenCL] Removes -D_GLIBCXX_USE_CXX11_ABI=0 as it breaks default compiler setup for Ubuntu 16.04 * Revert "[OpenCL] kernel_estimator_test.py assertEqual-> assertAlmostEqual due to floating point representation on the device" This reverts commit 06c50c0a485f40c30a436f02c3fa7794e370c49d. * [OpenCL] CPU allocator is a singleton we should not delete it

googlebot added the cla: no label Apr 10, 2017

lukeiwanski changed the title ~~[OpenCL] Implementation improvements (#22)~~ [OpenCL] Implementation improvements Apr 10, 2017

ville-k and others added 15 commits April 11, 2017 00:54

Register SYCL implementations for random ops

b7191a1

Avoid functions that might not be defined on SYCL device (#51)

f5b490f

* Avoid functions that might not be defined on SYCL device * Simplify by using Eigen math functions

[Build] Use gcc/g++ as a host compiler to avoid tensorflow#8394 (#54)

bb02931

[OpenCL] Fixes an issue caused by switch to aligned allocator for syc…

0bcfd46

…l buffer (#53)

[OpenCL] RandomGamma has no GPU friendly implementation (#57)

d308d7b

Register batch normalization kernels for OpenCL (#61)

08ccdc0

[OpenCL] Compatibility fixes for TensorFlow 1.1.0-rc1

8ce6f00

[OpenCL] Fixes Scatter Op

83719df

[OpenCL] Implements BatchMatmul Op for SYCL

2705d6d

Lowercase the device name when GPU or SYCL returned

3351426

[OpenCL] kernel_estimator_test.py assertEqual-> assertAlmostEqual due…

06c50c0

… to floating point representation on the device

[Eigen] Version bump

e093efa

lukeiwanski force-pushed the opencl_improvements branch from d00cb70 to e093efa Compare April 11, 2017 00:14

yifeif added cla: yes and removed cla: no labels Apr 11, 2017

Merge remote-tracking branch 'upstream/master' into opencl_improvements

6509298

googlebot added cla: no and removed cla: yes labels Apr 14, 2017

Merge branch 'master' into opencl_improvements

242cbda

Luke Iwanski added 3 commits May 5, 2017 21:17

Removes debug print

19fc923

Removes not needed partial specialisation

32edeab

[OpenCL] Registers ScatterNdFunctor for SYCL device

7f24669

rodburns mentioned this pull request May 8, 2017

More than one SYCL device supported? benoitsteiner/tensorflow-opencl#68

Open

lukeiwanski force-pushed the opencl_improvements branch from 53d50fe to 7f24669 Compare May 9, 2017 11:44

Luke Iwanski and others added 4 commits May 12, 2017 16:13

Merge remote-tracking branch 'upstream/master' into opencl_improvements

d7fbecf

Merge remote-tracking branch 'upstream/master' into opencl_improvements

a42b95a

[OpenCL] Make it compile

1d94291

[OpenCL] Follow gpu_device changes

ea2114a

rmlarsen added stat:awaiting response Status - Awaiting response from author stat:awaiting tensorflower Status - Awaiting response from tensorflower and removed stat:awaiting response Status - Awaiting response from author labels May 16, 2017

[OpenCL] Adds cxx_builtin_include_directory for python lib

3d422b6

Fixes bazels missing undeclared inclusions that appeared after merge with TensorFlow upstream

[OpenCL] Fixes Constant Op

c754c91

gunan reviewed May 26, 2017

View reviewed changes

Luke Iwanski added 4 commits May 29, 2017 10:40

[OpenCL] gXX-4.8 -> gXX

50197b9

[OpenCL] Removes -D_GLIBCXX_USE_CXX11_ABI=0 as it breaks default comp…

d777320

…iler setup for Ubuntu 16.04

Revert "[OpenCL] kernel_estimator_test.py assertEqual-> assertAlmostE…

1a60452

…qual due to floating point representation on the device" This reverts commit 06c50c0.

[OpenCL] CPU allocator is a singleton we should not delete it

4bd72b1

benoitsteiner approved these changes May 31, 2017

View reviewed changes

benoitsteiner merged commit fe589d9 into tensorflow:master May 31, 2017

lukeiwanski deleted the opencl_improvements branch August 3, 2017 10:40

[OpenCL] Implementation improvements #9117

[OpenCL] Implementation improvements #9117

lukeiwanski commented Apr 10, 2017 •

edited

tensorflow-jenkins commented Apr 10, 2017

googlebot commented Apr 10, 2017

lukeiwanski commented Apr 10, 2017

drpngx commented Apr 10, 2017

ville-k commented Apr 11, 2017

Zakor94 commented Apr 11, 2017

tensorflow-jenkins commented May 11, 2017

rmlarsen commented May 16, 2017

rmlarsen commented May 16, 2017

lukeiwanski commented May 17, 2017 •

edited

rmlarsen commented May 17, 2017

gunan May 26, 2017

lukeiwanski May 29, 2017

gunan May 26, 2017

gunan May 26, 2017

lukeiwanski May 29, 2017

gunan commented May 26, 2017

benoitsteiner commented May 31, 2017

[OpenCL] Implementation improvements #9117

[OpenCL] Implementation improvements #9117

Conversation

lukeiwanski commented Apr 10, 2017 • edited

tensorflow-jenkins commented Apr 10, 2017

googlebot commented Apr 10, 2017

lukeiwanski commented Apr 10, 2017

drpngx commented Apr 10, 2017

ville-k commented Apr 11, 2017

Zakor94 commented Apr 11, 2017

tensorflow-jenkins commented May 11, 2017

rmlarsen commented May 16, 2017

rmlarsen commented May 16, 2017

lukeiwanski commented May 17, 2017 • edited

rmlarsen commented May 17, 2017

gunan May 26, 2017

Choose a reason for hiding this comment

lukeiwanski May 29, 2017

Choose a reason for hiding this comment

gunan May 26, 2017

Choose a reason for hiding this comment

gunan May 26, 2017

Choose a reason for hiding this comment

lukeiwanski May 29, 2017

Choose a reason for hiding this comment

gunan commented May 26, 2017

benoitsteiner commented May 31, 2017

lukeiwanski commented Apr 10, 2017 •

edited

lukeiwanski commented May 17, 2017 •

edited