Skip to content

use --no-deps when installing .whl in TensorFlow easyblock if extension are being installed, move test run to sanity check#1537

Merged
akesandgren merged 4 commits intoeasybuilders:developfrom
boegel:TF_fixes
Oct 7, 2018
Merged

use --no-deps when installing .whl in TensorFlow easyblock if extension are being installed, move test run to sanity check#1537
akesandgren merged 4 commits intoeasybuilders:developfrom
boegel:TF_fixes

Conversation

@boegel
Copy link
Copy Markdown
Member

@boegel boegel commented Oct 2, 2018

This fixes the problem reported by @agijsberts in #1536, by also using --no-deps in the pip install command.

This change implies that all additional Python packages required by TensorFlow that are not available in the Python installation (e.g. numpy is) are included in the easyconfig file as extensions, so all TensorFlow easyconfig files that use this easyblock will need to be fixed...

Two other changes are also required:

  • running the MNIST tutorial example as a test now must be done in the sanity check, since not all extension are available yet after the pip install of the generated .whl; the comment claiming that this couldn't be done in the sanity check step was incorrect, since the build dir isn't cleaned up yet

  • creating of google/__init__.py now doesn't make sense anymore since the protobuf Python package will not be installed automagically anymore (instead, it should be listed as a dependency in the TensorFlow easyconfig file)

… test run to sanity check, don't create google/__init__.py in TF install dir
@boegel boegel changed the title also use --no-deps when installing .whl in TensorFlow easyblock, move test run to sanity check, don't create google/__init__.py in TF install dir use --no-deps when installing .whl in TensorFlow easyblock if extension are being installed, move test run to sanity check Oct 6, 2018
@boegel boegel requested a review from akesandgren October 6, 2018 17:08
@boegel
Copy link
Copy Markdown
Member Author

boegel commented Oct 6, 2018

@akesandgren After some further testing (& thinking), I've made a couple of changes:

  • prepare for installing Python packages as extensions

  • only use --no-deps when the TensorFlow easyconfig also installs extensions, which should effectively restore backward compatibility (i.e. we can only fix the auto-downloading of Python packages for the latest TensorFlow version without breaking older existing easyconfigs)

  • restore the touch on google/__init__.py, but only when it's really needed (i.e. when the google/protobuf dir exists)

Are you up for reviewing this & retesting easybuilders/easybuild-easyconfigs#6940 (which I also changed to use protobuf 3.6.0, based on the feedback by @agijsberts)?

@akesandgren
Copy link
Copy Markdown
Contributor

Working on Amber at the moment, and going to GTC next week so it might take a while before I have time.
Unless I have enough airport time...

Copy link
Copy Markdown
Contributor

@akesandgren akesandgren left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These changes look good and fixes the potential problems.

@boegel
Copy link
Copy Markdown
Member Author

boegel commented Oct 7, 2018

I've verified that this works both with the updated easyconfigs for TensorFlow 1.10.x (cfr. easybuilders/easybuild-easyconfigs#6940) and for existing untouched TensorFlow easyconfigs, so it's good to go imho...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants