Skip to content

fixes for TensorFlow easyblocks w.r.t. Bazel build options & __init__ in top-level google-protobuf package dir#1843

Merged
boegel merged 2 commits intoeasybuilders:developfrom
Flamefire:tensorflow
Nov 5, 2019
Merged

fixes for TensorFlow easyblocks w.r.t. Bazel build options & __init__ in top-level google-protobuf package dir#1843
boegel merged 2 commits intoeasybuilders:developfrom
Flamefire:tensorflow

Conversation

@Flamefire
Copy link
Copy Markdown
Contributor

@Flamefire Flamefire commented Nov 5, 2019

Fix the warning

WARNING: The following configs were expanded more than once: [v2, cuda, using_cuda]. For repeatable flags, repeats are counted twice and may lead to unexpected behavior.

in TF 2 (see 7f62330#diff-925e7942309e22e4c64cb2d99b50c206) and remove google/__init__.py for Python 3.3 (see easybuilders/easybuild-easyconfigs#9207)

@Flamefire
[Tensorflow] Fix multiple config warning
e078b52 
--config=v2 was only required for TF 2.0.0-rcx for which there is no EC
--config=cuda is set in configure.py if CUDA support is requested
houndci-bot
houndci-bot reviewed Nov 5, 2019
View reviewed changes
Comment thread easybuild/easyblocks/t/tensorflow.py
@boegel boegel changed the title Fix for Tensorflow fixes for TensorFlow easyblocks w.r.t. Bazel build options & __init__ in top-level google-protobuf package dir Nov 5, 2019
@boegel boegel added the bug fix label Nov 5, 2019
@boegel boegel added this to the next release (4.0.2?) milestone Nov 5, 2019
@boegel
Copy link
Copy Markdown
Member

boegel commented Nov 5, 2019

Looks good on first sight, I'll test this with a couple of existing TensorFlow easyconfigs to make sure there are no regressions introduced...

@Flamefire
Copy link
Copy Markdown
Contributor Author

Flamefire commented Nov 5, 2019

Highly appreciated!

Note that I "removed support" for Tensorflow 2 RCs which did require the --config=v2, but including that would add a lot of trash to the source (you need the option for 2 RC but not 2 and not 1 and not >=2.0.1) because (in difference to SemVer) 2.0.0-rc1>=2.0.0. With the release of the 2.0.0 I'd rather not include that code.

@boegel
Copy link
Copy Markdown
Member

boegel commented Nov 5, 2019

Agreed, I don't think having no support for 2.0.0-rc* is an issue. That also explains why the --config=v2 was there (I updated the easyblock using 2.0.0-rc2)

@boegel
Copy link
Copy Markdown
Member

boegel commented Nov 5, 2019

Tested with a variety of existing TensorFlow easyconfigs, including:

  • TensorFlow-2.0.0-foss-2019a-Python-3.7.2.eb
  • TensorFlow-1.14.0-foss-2019a-Python-3.7.2.eb
  • TensorFlow-1.13.1-foss-2018b-Python-3.6.6.eb
  • TensorFlow-1.8.0-intel-2018a-Python-3.6.4.eb
  • TensorFlow-1.3.0-intel-2017a-Python-2.7.13.eb

Works as expected, so no reason to hold this back. Thanks a lot for the fixes @Flamefire!

@boegel boegel merged commit 24b076f into easybuilders:develop Nov 5, 2019
@Flamefire Flamefire deleted the tensorflow branch November 5, 2019 12:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@boegel boegel boegel approved these changes

+1 more reviewer

@houndci-bot houndci-bot houndci-bot left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

bug fix

Projects

None yet

Milestone

4.1.0

Development

Successfully merging this pull request may close these issues.

3 participants

@Flamefire @boegel @houndci-bot