Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Raspberry PI build #11675

Merged
merged 21 commits into from
Jul 26, 2017
Merged

Add Raspberry PI build #11675

merged 21 commits into from
Jul 26, 2017

Conversation

ebrevdo
Copy link
Contributor

@ebrevdo ebrevdo commented Jul 21, 2017

Does what it says on the box; follow instructions in third_party/toolchains/cpus/arm/build_raspberry_pi.sh

ebrevdo added 3 commits July 21, 2017 13:01
To build:

./configure

bazel build --sandbox_debug -c opt --copt=-march=armv6 --copt=-mfpu=vfp \
  --copt=-funsafe-math-optimizations --copt=-ftree-vectorize \
  --copt=-fomit-frame-pointer --cpu=armeabi \
  --crosstool_top=@local_config_arm_compiler//:toolchain \
  --verbose_failures --build_python_zip ...targets...
update platform.h and crosstool template for define RASPBERRY_PI.
@ebrevdo ebrevdo requested review from gunan and petewarden July 21, 2017 22:09
Copy link
Contributor

@petewarden petewarden left a comment

Choose a reason for hiding this comment

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

One minor comment.

// Since there's no macro for the Raspberry Pi, assume we're on a mobile
// platform if we're compiling for the ARM CPU.
// Require an outside macro to tell us if we're building for Raspberry Pi.
#if !defined(RASPBERRY_PI)
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you add this define to the Makefile for the Pi too? Hopefully that will go away eventually, but for now it would be good to keep them in sync.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@ebrevdo ebrevdo requested review from satok16 and wolffg as code owners July 21, 2017 23:18
# sudo tee -a /etc/apt/sources.list.d/armhf.list
# # Ignore errors about missing armhf packages in other repos.
# sudo apt-get update
# sudo apt-get install libpython-all-dev:armhf
Copy link
Contributor

Choose a reason for hiding this comment

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

I had to use aptitude instead of apt-get on my Goobuntu machine, since otherwise I saw:

Some packages could not be installed. This may mean that you have
requested an impossible situation or if you are using the unstable
distribution that some required packages have not yet been created
or been moved out of Incoming.
The following information may help to resolve the situation:

The following packages have unmet dependencies:
libpython-all-dev:armhf : Depends: libpython-dev:armhf (= 2.7.5-5ubuntu3) but it is not going to be installed
Depends: libpython2.7-dev:armhf but it is not going to be installed
E: Unable to correct problems, you have held broken packages.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Interesting. I updated the comment to suggest using aptitude instead.

It did work for you in the end though?

# To install the architecture includes for python on ubuntu trusty;
# run:
# sudo dpkg --add-architecture armhf
# echo "deb [arch=armhf] http://ports.ubuntu.com/ trusty main universe" \
Copy link
Contributor

Choose a reason for hiding this comment

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

I think there should be a pipe '|' in here?



# This performs an unsafe unzip operation - since zip files may have
# relative and absolute paths, it should only be executed on trusted files.
Copy link
Contributor

Choose a reason for hiding this comment

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

@meteorcloudy Can bazel handle zip files with symlinks in windows now?
Would this break windows bazel build (not currently run in presubmits)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

no; this part is only executed if you run with the --crosstool argument in the .sh script. otherwise none of these commands are executed.

@@ -0,0 +1,49 @@
package(default_visibility = ["//visibility:public"])
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we move this and its crosstool file under third_party/cpus/cc ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

moved to third_party/toolchains/cpus/arm.


def _unzip(repo_ctx, zip_file, strip_prefix):
_run_cmd(repo_ctx,
["unzip", repo_ctx.path(zip_file),
Copy link
Contributor

Choose a reason for hiding this comment

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

should we protect this with an OS check?
How platform independent is unzip command?
macos should be OK, but what would this do on windows @meteorcloudy

Copy link
Contributor Author

Choose a reason for hiding this comment

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

isn't this going to be running inside a sh shell within windows? i.e., if mkdir and cp work, unzip should too...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

(anyway; i'm pretty sure that the crosstool compiler won't run on windows anyway...)

@@ -0,0 +1,81 @@
package(default_visibility = ['//visibility:public'])
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this file also be under third_party/ ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

moved.

@petewarden
Copy link
Contributor

I've got a gist showing how to build this on a clean docker image, and I've verified the resulting wheel works on my Pi!
https://gist.github.com/petewarden/6aa9650fe72d88889d24efd76598f8cb

Adding in @samjabrahams to the thread too, since he's the expert on Pi Python building.

@samjabrahams
Copy link
Contributor

Very cool; I'm really happy to see this getting added in. I won't be able to test it personally until August (due to all my RPis being in storage), but I'm excited to give it a shot as soon as I can.

Couple questions/comments:

  • We're targeting the armv6 instruction set- is the plan to have Raspberry Pi 1/Zero support?
  • We're not using NEON in the current build (I'm assuming because of the ARMv6 target), but I think it's provides pretty significant optimizations (@petewarden knows more about the details of this than myself). Hopefully, the difference in speed between this and a build with NEON is negligible, but if not, would it be possible to consider adding an option to the build to use it? Granted, this might cause some headaches in the various scripts and would require using GCC 4.8 (I think we could swap in this gnueabi, but it's older and also not my area of expertise).

Finally, it's satisfying that #3469 is getting addressed almost exactly a year to the day after it was opened :D

@petewarden petewarden mentioned this pull request Jul 23, 2017
@ebrevdo
Copy link
Contributor Author

ebrevdo commented Jul 24, 2017

Anything else left to fix for this? @petewarden should i move any of your docker comments into my PR; or do you want to have a separate .sh file after this one is pushed? I'll be OOO for 2 weeks starting Wednesday, so any changes should happen by tomorrow.

@ebrevdo
Copy link
Contributor Author

ebrevdo commented Jul 24, 2017

@samjabrahams happy to help. i am happy to add an argument or some comments in the .sh file for making an optimized build for arm7; or alternatively building for arm7 + neon and showing how to make an arm6 build (probably leaning towards this latter approach).

@petewarden
Copy link
Contributor

@ebrevdo I've got an outstanding PR to add ARM7/NEON support and update the comments here, can you take a look? ebrevdo#3

@gunan
Copy link
Contributor

gunan commented Jul 25, 2017

Looks good after a rebase.
Test failures do not look related to me, we can rerun them after the rebase.

Added an extra system include
@ebrevdo
Copy link
Contributor Author

ebrevdo commented Jul 25, 2017

Added @petewarden's changes to make default build armv7/NEON. This required upgrading the version of Eigen we use. @benoitsteiner is this OK?

@petewarden
Copy link
Contributor

@benoitsteiner I believe this addresses issue #9697

@vrv
Copy link

vrv commented Jul 26, 2017

@tensorflow-jenkins test this please

@petewarden
Copy link
Contributor

The remaining MacOS failures look like timeouts or flakes.

@vrv vrv merged commit b244912 into tensorflow:master Jul 26, 2017
@samjabrahams
Copy link
Contributor

Awesome! Once I find some time, I'll note in my repo that official support is available and link to the relevant instructions.

@ebrevdo
Copy link
Contributor Author

ebrevdo commented Jul 27, 2017 via email

@petewarden
Copy link
Contributor

That's correct. We're blocked on #9697 for the Pi 3 build, and once that's done I also need to create a Jenkins CI build to run nightly, so we'll have automatically generated binaries for the wheel. I hope to get to that soon.

@ptone
Copy link

ptone commented Jul 29, 2017

Once done, will these wheels be available on PyPI for pip installation?

https://pypi.python.org/pypi/tensorflow

@ebrevdo
Copy link
Contributor Author

ebrevdo commented Jul 30, 2017 via email

@ptone
Copy link

ptone commented Jul 30, 2017

I believe the grpc team sorted this out somehow - I can forward you an internal thread.

https://pypi.python.org/pypi/grpcio/1.4.0

@ptone
Copy link

ptone commented Jul 30, 2017

I'm not sure what happened, (git-gone-funky?), but the update to the Eigen version did not make it into master:
c848583#diff-455a4c7f8e22d7c514e8c2caa27506c5R162

As of the current master commit the eigen archive is still set to rev f3a22f35b044

@petewarden
Copy link
Contributor

@ptone Sorry, I should have been more clear in the comments. We're blocked on updating Eigen (#9697) because the newer Eigen version breaks on MacOS. We'll need to see if other Eigen versions work on both, but I'm not working on that at the moment.

If anyone wants, they can update the Eigen hash to the Pi 3-capable one locally and build the wheel, but we won't be able to support it officially until the MacOS issue is resolved.

@AnchoretBaladev
Copy link

Hello @petewarden I'm trying to cross compile tensorflow for Raspberry PI 3 from Ubuntu 16.04LTS using the "build_raspberry_pi.sh" present inside "/tensorflow/tools/ci_build/pi/" with the following steps:-
1:- Installing Bazel
2:- Cloning the Tensorflow and runnining the "build_raspberry_pi.sh"
but not getting success!!
Can you please guide me what are the needed steps to follow to cross compile!!! Thanks in Advance!!

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.

8 participants