add easyblock for TensorFlow (REVIEW) by boegel · Pull Request #1287 · easybuilders/easybuild-easyblocks

boegel · 2017-11-09T13:49:45Z

With this easyblock, I'm able to build & install TensorFlow 1.4.0 from source using foss/2017b and Python 3.6.3 (cfr. easybuilders/easybuild-easyconfigs#5318).

Further testing is required, especially w.r.t. building with Intel compilers on top of Intel MKL & GPU support (i.e., including CUDA & cuDNN as dependencies).

One thing not taking care of now is avoiding that the installation procedure pulls in a whole bunch of dependencies itself, it seems like Bazel doesn't have good support for providing the dependencies via some other way (meaning that a lot of patching is probably required to avoid that they're automagically installed).

Also, we can avoid the need for run_cmd_qa by setting $TF_* environment variables that are picked up by configure, it may be worth while switching to that approach.

pforai · 2017-11-09T14:32:57Z

+1 on the env vars! Seeing strange things with the interactive installer vs running it under eb.

… MKL light instead

…Flow build command

…nt variables used by configure.py script

…nes, add debugging

pforai · 2017-11-29T08:28:48Z

The env vars based block doesn't hang any more since its no long based on the qanda block 👍

damianam

I am not familiar with TF, so I'd suggest to look for a maintainer more acquainted with it for an additional review.

damianam · 2017-12-08T14:59:01Z

easybuild/easyblocks/t/tensorflow.py

+
+        tmpdir = tempfile.mkdtemp(suffix='-bazel-configure')
+
+        # create wrapper for icc to make sure location of license server is available...


Explain here that bazel sandboxes the environment and it is then when INTEL_LICENSE_FILE gets lost?

ok, will do

damianam · 2017-12-08T15:22:51Z

easybuild/easyblocks/t/tensorflow.py

+        cuda_root = get_software_root('CUDA')
+        cudnn_root = get_software_root('cuDNN')
+        jemalloc_root = get_software_root('jemalloc')
+        opencl_root = get_software_root('OpenCL')


There isn't any OpenCL easyconfig as far as I can see. Besides, the OpenCL runtime for intel processors needs root permissions, so it won't be made available any time soon through EB.

Well, ok, but I'll leave this in anyway, to avoid hardcoding building without OpenCL support.

At least this way there is an option to built TF with OpenCL support (for example via http://easybuild.readthedocs.io/en/latest/Using_external_modules.html).

damianam · 2017-12-08T15:42:43Z

easybuild/easyblocks/t/tensorflow.py

+        cmd.append(self.cfg['buildopts'])
+
+        # pass through environment variables that may specify location of license file for Intel compilers
+        for key in []:


I have the feeling that this shouldn't be an empty list....

Woops, yes, I did the for for $INTEL_LICENSE_FILE through this initially, but it turns out that doesn't actually work...

damianam · 2017-12-08T15:43:52Z

easybuild/easyblocks/t/tensorflow.py

+            # using the full Intel MKL doesn't work without additional effort...
+            cmd.extend(['--config=mkl'])
+
+        cmd.append('//tensorflow/tools/pip_package:build_pip_package')


I am not sure what this does, but I going to guess that the double slash is a typo.

No, it's not, that's how Bazel works... See https://www.tensorflow.org/install/install_sources#build_the_pip_package .

…means of test, in install step

…nto TensorFlow

…easyblock

Added preconfigopts, configopts and a few other things in tensorflow …

boegel · 2018-02-12T21:50:30Z

@damianam @akesandgren I gave this another, hopefully final, review, I think this is good to go in now...
If the both of you agree (please take a couple of minutes to give the whole thing another view), this is good to merge by one of the @easybuilders/easybuild-easyblocks-maintainers, to be included with the upcoming EasyBuild v3.5.2 (ETA end of Feb'18).

akesandgren · 2018-02-13T07:41:14Z

easybuild/easyblocks/t/tensorflow.py

+        extra_vars = {
+            # see https://developer.nvidia.com/cuda-gpus
+            'cuda_compute_capabilities': [[], "List of CUDA compute capabilities to build with", CUSTOM],
+            'with_jemalloc': [True, "Make TensorFlow use jemalloc", CUSTOM],


Wouldn't it be better to use jemalloc only when it is present as a dependency and remove this with_jemalloc thingie?

Currently we have no way to let Bazel pick up on self-provided dependencies, but we needed a way to disable it pulling in and building jemalloc itself (which is required on CentOS 6).

This is something to improve on in the future, but I think we'll have to wait for improvements in Bazel itself before this is feasible without heavy patching, it's a huge PITA right now to make that work.

This easyblock is a starting point, definitely not the end of the road...

Ok, good enough for me then.
Awaiting merge so i can redo the builds...

boegel · 2018-02-13T07:44:19Z

@JackPerdue I guess we can relax the binutils requirement, sure, it's a bit strict right now... It's just that I was making the assumption here that a 'modern' EasyBuild toolchain is used, where we include our own binutils.
Is this a blocker for you?

akesandgren · 2018-02-13T07:53:28Z

Looks good enough to go for me.

boegel · 2018-02-13T08:30:05Z

(re)tested with:

TensorFlow 1.4.0 with foss/2017b (see {lib}[foss/2017b] TensorFlow v1.4.0 w/ Python 3.6.3 (built from source) easybuild-easyconfigs#5318) and intel/2017b (see https://github.com/easybuilders/easybuild-easyconfigs/pull5499)
TensorFlow 1.4.1 with foss/2017b (see {lib}[foss/2017b] TensorFlow v1.4.1 w/ Python 3.6.3 (built from source) easybuild-easyconfigs#5814)
TensorFlow 1.5.0 with foss/2017b (see {lib}[foss/2017b] TensorFlow v1.5.0 w/ Python 3.6.3 (built from source) easybuild-easyconfigs#5819)

pending: TensorFlow 1.5.0 with goolfc/2017b (by @akesandgren, see easybuilders/easybuild-easyconfigs#5769)

JackPerdue · 2018-02-13T14:48:02Z

@boegel no blocker here....digging further I see I have similar problems with Bazel. So, for now, I've hacked together a binutils-system using bundle.py which only does:

modextravars = {'EBROOTBINUTILS': '/usr'}

Seems to work since it gets listed last in module. That causes bazel.py to do a bunch of other stufff like testing for dwp who's absence on RHEL6 didn't show up until building TensorFlow. Unfortunately, that hasn't solved the other problem with Bazel in that it wants to use the system GCC instead of the EB one. Running "strings" on bazel still shows:

GCC: (GNU) 4.8.5 20150623 (Red Hat 4.8.5-16)
GCC: (GNU) 6.3.0

and 4.8.5 is what gets used by Bazel when building TF which breaks it (e.g. trying to use /usr/lib/gcc/x86_64-redhat-linux/4.8.5/include/stdint.h). I tried rebuilding Bazel with all gcc RPMs removed but somehow it still picks it up. So to build TF, I still have to remove system gcc (which would not be my first choice). [note, still use Bazel block and config from EB 3.5.1]

Anyway.... not a blocker for me... lets get something out there and see who else has problems/fixes.

boegel · 2018-02-19T16:46:05Z

@wpoely86 Do you see any reason not to merge this?

changes done

wpoely86 · 2018-02-19T19:35:37Z

No, I forgot to click the shiny green button 😉

boegel · 2018-02-20T07:51:26Z

🎉

add easyblock for TensorFlow (WIP)

f7473ed

boegel added the new label Nov 9, 2017

boegel added this to the 3.5.0 milestone Nov 9, 2017

boegel mentioned this pull request Nov 9, 2017

{lib}[foss/2017b] TensorFlow v1.4.0 w/ Python 3.6.3 (built from source) easybuilders/easybuild-easyconfigs#5318

Merged

boegel added 17 commits November 9, 2017 16:13

check for mkl-dnn rather than imkl to build TensorFlow with MKL support

ccc037d

complete Q&A for MPI in TensorFlow easyblock

0046341

stick to using full Intel MKL for now rather than using mkl-dnn

4e57108

fix missing string completion in test step of TensorFlow easyblock

2c974a7

also patch cxx_builtin_include_directory lines in CROSSTOOL* scripts

505dbf6

use 'pip install --upgrade' to ensure that correct *.whl is installed

262b113

only run 'core' and 'python' test subsuites

71f9550

create wrapper for icc to make sure $INTEL_LICENSE_FILE is set...

ed74478

use 'pip install --ignore-installed' rather than 'pip install --update'

dc1e9a6

don't specify Intel MKL install prefix, let TensorFlow download Intel…

8b5528d

… MKL light instead

also include CUDA's include & lib64 when patching CROSSTOOL scripts

de1daab

don't strictly require CUDA in TensorFlow easyblock

241307b

add missing --compilation_mode=opt (equivalent with -c opt) to Tensor…

59d12c2

…Flow build command

add custom 'with_mkl_dnn' easyconfig parameter for TensorFlow

403f182

avoid interactive configure for TensorFlow by setting magic environme…

12fc85f

…nt variables used by configure.py script

determine value for nvcc host compiler via $CC

c688395

handle -fPIC, fix indent for patched cxx_builtin_include_directory li…

c45e0d9

…nes, add debugging

alalazo mentioned this pull request Dec 6, 2017

Features/tensorflow spack/spack#3244

Closed

3 tasks

boegel mentioned this pull request Dec 7, 2017

use of ccache leads to compilation problems when Fortran modules are involved easybuilders/easybuild-framework#2080

Open

only run 'core' TensorFlow tests, but exclude 'grpc' tests

f2c7d59

damianam requested changes Dec 8, 2017

View reviewed changes

fix remarks for TensorFlow easyblock, run MNIST tutorial examples by …

50475db

…means of test, in install step

boegel force-pushed the TensorFlow branch from 499b672 to 50475db Compare December 10, 2017 12:49

boegel changed the title ~~add easyblock for TensorFlow (WIP)~~ add easyblock for TensorFlow (REVIEW) Dec 10, 2017

Merge branch 'TensorFlow' of github.com:boegel/easybuild-easyblocks i…

b060ba5

…nto TensorFlow

boegel mentioned this pull request Feb 8, 2018

Proposal: Making the cmake build distribution friendly tensorflow/tensorflow#13061

Closed

Damian Alvarez and others added 4 commits February 8, 2018 18:01

Added preconfigopts, configopts and a few other things in tensorflow …

fa66b37

…easyblock

Cleanup

b44da77

Control the amount of jobs that bazel expands

584965c

Merge pull request #20 from damianam/tensorflow

0f60bda

Added preconfigopts, configopts and a few other things in tensorflow …

boegel mentioned this pull request Feb 12, 2018

{lib}[foss/2017b] TensorFlow v1.4.1 w/ Python 3.6.3 (built from source) easybuilders/easybuild-easyconfigs#5814

Merged

don't use mkl-dnn when cuDNN is included as dependency

960e97c

boegel changed the title ~~add easyblock for TensorFlow (WIP)~~ add easyblock for TensorFlow (REVIEW) Feb 12, 2018

final minor enhancements + additional comments

233fe49

This was referenced Feb 12, 2018

{lib}[foss/2017b] TensorFlow v1.5.0 w/ Python 3.6.3 (built from source) easybuilders/easybuild-easyconfigs#5819

Merged

switch to using TensorFlow as backend for recent versions of Keras easybuilders/easybuild-easyconfigs#5821

Merged

akesandgren reviewed Feb 13, 2018

View reviewed changes

boegel mentioned this pull request Feb 13, 2018

{lib}[goolfc/2017b] TensorFlow 1.5.0 w/ Python 3.6.3 (built from source) easybuilders/easybuild-easyconfigs#5769

Merged

wpoely86 approved these changes Feb 15, 2018

View reviewed changes

wpoely86 merged commit b264b72 into easybuilders:develop Feb 19, 2018

boegel deleted the TensorFlow branch February 20, 2018 07:51

boegel mentioned this pull request Feb 20, 2018

{devel}[foss/2016b] TensorFlow v1.0.1, SWIG v3.0.10, wheel v0.30.0a0, ... easybuilders/easybuild-easyconfigs#4412

Closed

boegel mentioned this pull request Mar 1, 2018

fix installation of TensorFlow 1.3 via binary wheel after introducing TensorFlow easyblock easybuilders/easybuild-easyconfigs#5938

Merged

boegel mentioned this pull request Mar 14, 2018

TensorFlow 1.6.0 fix: use the absolute path for the C compiler when compiling with GPU support #1386

Merged

boegel mentioned this pull request Oct 1, 2018

Use of --ignore-installed in TensorFlow's step_install #1536

Closed


		tmpdir = tempfile.mkdtemp(suffix='-bazel-configure')

		# create wrapper for icc to make sure location of license server is available...

Conversation

boegel commented Nov 9, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pforai commented Nov 9, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pforai commented Nov 29, 2017

Uh oh!

damianam left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

boegel commented Feb 12, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

boegel commented Feb 13, 2018

Uh oh!

akesandgren commented Feb 13, 2018

Uh oh!

boegel commented Feb 13, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JackPerdue commented Feb 13, 2018

Uh oh!

boegel commented Feb 19, 2018

Uh oh!

wpoely86 commented Feb 19, 2018

Uh oh!

boegel commented Feb 20, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

boegel commented Nov 9, 2017 •

edited

Loading

pforai commented Nov 9, 2017 •

edited

Loading

boegel commented Feb 13, 2018 •

edited

Loading