Skip to content

Conversation

@Flamefire
Copy link
Contributor

@Flamefire Flamefire commented Aug 19, 2020

(created using eb --new-pr)

The method of installing protobuf-python from the github (sub-)tree contained in the released protobuf-python.tar.gz requires using setup.py instead of pip and installs an egg folder instead of a "regular installation".
This conflicts with other google packages which install regular site-packages/google/foo trees resulting in protobuf not being found. This happens e.g. in TensorFlow when trying to use this EC.

Additionally the C++ extension is not build causing decreased performance in some scenarios (not sure which, didn't dig deeper but it is mentioned prominently in their readme)

This PR makes it use the PyPI source release which is installable by pip because it contains precompiled protobuf files (pip install on the source tree fails due to relative paths)

Problem: As it makes sense to name the downloaded file protobuf-python.tar.gz (via the template) it will yield in a wrong checksum if the EC was installed before and requires a forced redownload. IMO this is acceptable as the existing EC is relatively new and likely not used to often

@Flamefire Flamefire force-pushed the 20200819163158_new_pr_protobuf-python3100 branch from 4bd07e6 to d7f73a6 Compare August 19, 2020 14:49
@Flamefire
Copy link
Contributor Author

Test report by @Flamefire
SUCCESS
Build succeeded for 2 out of 2 (2 easyconfigs in this PR)
taurusml11 - Linux RHEL 7.6, POWER, 8335-GTX, Python 2.7.5
See https://gist.github.com/3c4fd27b8b8680ab3f88cfeb182c33a3 for a full test report.

@Flamefire
Copy link
Contributor Author

Test report by @Flamefire
SUCCESS
Build succeeded for 2 out of 2 (2 easyconfigs in this PR)
taurusi5126.taurus.hrsk.tu-dresden.de - Linux RHEL 7.8, x86_64, Intel(R) Xeon(R) CPU E5-2680 v3 @ 2.50GHz, Python 2.7.5
See https://gist.github.com/37b610c0866dd15d790f3ecbf519a27c for a full test report.

@boegel
Copy link
Member

boegel commented Aug 20, 2020

@boegelbot please test @ generoso

@boegelbot
Copy link
Collaborator

@boegel: Request for testing this PR well received on generoso

PR test command 'EB_PR=11143 EB_ARGS= /apps/slurm/default/bin/sbatch --job-name test_PR_11143 ~/boegelbot/eb_from_pr_upload_generoso.sh' executed!

  • exit code: 0
  • output:
Submitted batch job 5092

Test results coming soon (I hope)...

Details

- notification for comment with ID 677652902 processed

Message to humans: this is just bookkeeping information for me,
it is of no use to you (unless you think I have a bug, which I don't).

@boegel
Copy link
Member

boegel commented Aug 20, 2020

Installation will fail if source tarball was downloaded with original easyconfig, but there's little we can do to prevent that...

Easy to fix with eb --from-pr 11143 --force-download

@boegel boegel changed the title Use pip to install protobuf-python in 2019b toolchain Use pip to install protobuf-python in 2019b toolchain (requires re-downloading source tarball!) Aug 20, 2020
Copy link
Member

@boegel boegel left a comment

Choose a reason for hiding this comment

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

lgtm

@boegelbot
Copy link
Collaborator

Test report by @boegelbot
SUCCESS
Build succeeded for 2 out of 2 (2 easyconfigs in this PR)
generoso-x-3 - Linux centos linux 8.2.2004, x86_64, Intel(R) Xeon(R) CPU E5-2667 v3 @ 3.20GHz (haswell), Python 3.6.8
See https://gist.github.com/3c14e90cc8965f26c0ea71e07f1e2ce3 for a full test report.

@boegel
Copy link
Member

boegel commented Aug 20, 2020

Test report by @boegel
SUCCESS
Build succeeded for 2 out of 2 (2 easyconfigs in this PR)
node3100.skitty.os - Linux centos linux 7.8.2003, x86_64, Intel(R) Xeon(R) Gold 6140 CPU @ 2.30GHz (skylake_avx512), Python 3.6.8
See https://gist.github.com/a6ff44f76e02da0486a7b16f53a360e1 for a full test report.

@boegel
Copy link
Member

boegel commented Aug 20, 2020

Test report by @boegel
SUCCESS
Build succeeded for 2 out of 2 (2 easyconfigs in this PR)
node3407.kirlia.os - Linux centos linux 7.8.2003, x86_64, Intel(R) Xeon(R) Gold 6240 CPU @ 2.60GHz (cascadelake), Python 2.7.5
See https://gist.github.com/65a81995ebcc883eceb1e8dccdde87ef for a full test report.

@boegel
Copy link
Member

boegel commented Aug 20, 2020

Test report by @boegel
SUCCESS
Build succeeded for 2 out of 2 (2 easyconfigs in this PR)
node2415.golett.os - Linux centos linux 7.8.2003, x86_64, Intel(R) Xeon(R) CPU E5-2680 v3 @ 2.50GHz (haswell), Python 2.7.5
See https://gist.github.com/092c7871161116840c2315cf6ce47d44 for a full test report.

@boegel
Copy link
Member

boegel commented Aug 20, 2020

Going in, thanks @Flamefire!

@boegel boegel merged commit 3338d08 into easybuilders:develop Aug 20, 2020
@Flamefire Flamefire deleted the 20200819163158_new_pr_protobuf-python3100 branch August 20, 2020 13:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants