Page MenuHomeFreeBSD

science/tensorflow: Computation using data flow graphs for scalable machine learning
ClosedPublic

Authored by amutu_amutu.com on Jun 14 2017, 3:53 AM.

Details

Summary

[NEW PORT] science/tensorflow: Computation using data flow graphs for scalable machine learning

TensorFlow is an open source software library for numerical computation using
data flow graphs. Nodes in the graph represent mathematical operations, while
the graph edges represent the multidimensional data arrays (tensors)
communicated between them. The flexible architecture allows you to deploy
computation to one or more CPUs or GPUs in a desktop, server, or mobile device
with a single API. TensorFlow was originally developed by researchers and
engineers working on the Google Brain Team within Google's Machine Intelligence
research organization for the purposes of conducting machine learning and deep
neural networks research, but the system is general enough to be applicable in
a wide variety of other domains as well.

WWW: https://www.tensorflow.org

PR:219609

Test Plan

portlint: no error; poudriere testport pass: 11.0R-amd64-py27, 11.0R-amd64-py3, 11.0R-i386-py27, 10.3R-amd64, 12current-amd64-py27

Note : this port depend on PR https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=219840 for build,and PR https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=220206 for runtime depend

There are tensorflow poudriere testport logs(11amd64,11i386,11amd64-py3,10.3amd64,12current):
103amd64-default.log
11amd64-default-py3.log
11amd64-default.log
11i386-default.log
12current-20170619-default.log

tensorflow functional test inputs steps:
from https://github.com/amutu/tensorflow-port/tree/master/test ,just download the *.py files, after install tensorflow,you can test it like this:

python2.7 ./mnist_deep.py,output will like this:
some warning...
can't determine number of CPU cores: assuming 4
can't determine number of CPU cores: assuming 4
step 0, training accuracy 0.1
step 0, training accuracy 0.1
step 100, training accuracy 0.82
step 200, training accuracy 0.94
...

python2.7 ./mnist_softmax.py,output will like this:
some warning...
can't determine number of CPU cores: assuming 4
can't determine number of CPU cores: assuming 4
0.9188

python2.7 ./linear_regression.py,output will like this:
some warning...
train loss: {'loss': 3.355257e-08, 'global_step': 1000}
eval loss: {'loss': 0.0025368345, 'global_step': 1000}

Diff Detail

Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 9872
Build 10307: arc lint + arc unit

Event Timeline

amutu_amutu.com edited the summary of this revision. (Show Details)Jun 14 2017, 3:59 AM
amutu_amutu.com edited the test plan for this revision. (Show Details)
mat added inline comments.Jun 14 2017, 8:19 AM
Makefile
21

Don't add commented out lines.

30

This is already in python_OLD_CMD, no need to add it again.

35

OSREL:R

amutu_amutu.com marked 3 inline comments as done.Jun 14 2017, 9:55 AM

Thanks mat,will fix

1.remove commented out lines.
2.remove python_OLD_CMD
3.use OSREL:R

1.update to tensorflow-1.2.0
2.use PATCHFILES to fetch large patch files
3.fix runtime depend

amutu_amutu.com edited the test plan for this revision. (Show Details)Jun 28 2017, 9:06 AM
amutu_amutu.com edited the test plan for this revision. (Show Details)Jun 28 2017, 11:05 AM
lwhsu added inline comments.Jun 28 2017, 11:12 AM
Makefile
22

I remember in previous diff only OSREL:R <= 10 needs devel/bazel_clang38, so now it requires this unconditionally?

lwhsu added inline comments.Jun 28 2017, 11:15 AM
distinfo
10

let's make these external patches versioned and be placed under tensorflow in distfiles directory, to prevent name collisions in the future.

Makefile
22

previous build tensorflow need mount /proc fs, because bazel need /proc to get its binary path. I make a patch to get rid of this and the patch added to bazel_clang38. So now tensorflow build with bazel_clang38 on all FreeBSD version without need /proc.

distinfo
10

in fact, the hash value in the PATCH_SITES is a git commit, so it is some meaning of version. but I am willing to change because version number is more readable.

PATCH_SITES= https://raw.githubusercontent.com/amutu/tensorflow-port/a730557780159372986f4fcae30f6e396d14060d/files/

lwhsu added inline comments.Jun 28 2017, 12:00 PM
Makefile
22

Got it, I still do not fully understand bazel_clang38 but we can work directly on it in https://bugs.freebsd.org/219840 . Using a customized bazel here is fine.

This one can be marked as done.

distinfo
10

The main point is trying to prevent file name collision in user's /usr/ports/distfiles directory in the future updates. What I suggest here is, putting them to a specified directory, such as /usr/ports/distfiles/tensorflow , and naming patch files like patch-WORKSPACE-some_hashcode or patch-WORKSPACE-v1.2.3. Using a subdirectory such as /usr/ports/distfiles/tensorflow/some_hashcode or /usr/ports/distfiles/tensorflow/v1.2.3 is also a possible way if renaming patch files is unpractical.

amutu_amutu.com marked 3 inline comments as done.Jun 28 2017, 12:30 PM
amutu_amutu.com added inline comments.
distinfo
10

Got it,thank you for your explanation!I have changed the external patch like this:

PATCH_SITES= https://raw.githubusercontent.com/amutu/tensorflow-port/v${PORTVERSION}/files/
PATCHFILES= patch-WORKSPACE-${PORTVERSION} patch-tensorflow_workspace.bzl-${PORTVERSION}

Will update the review patch later.

mat added inline comments.Jun 28 2017, 12:42 PM
Makefile
38

No need for empty variables.

40

This already exist, no need for it.

55–58

sed can change more than one file at a time, you should put this two files in the same call.

1.add version to external patch files
2.remove unnecessary code and merge sed cmd per mat@

amutu_amutu.com marked 3 inline comments as done and an inline comment as not done.Jun 28 2017, 1:30 PM
amutu_amutu.com added inline comments.
Makefile
30

fixed,thanks!

35

fixed, thanks!

38

fixed,thanks!

40

fixed,thanks!

55–58

fixed,thanks!

amutu_amutu.com marked 3 inline comments as done and an inline comment as not done.Jun 28 2017, 1:32 PM

I think I misread mat's comment about shebang fix,the SHEBANG_LANG should be removed and the SHEBANG_GLOB=*.py should not be removed, or the build will fail for not find python.
I run poudriere testport and now build ok.

amutu_amutu.com marked 5 inline comments as done.Jul 6 2017, 10:08 AM

Hi, may I ask, is this ready to commit? And who will do the commit?

lwhsu edited edge metadata.Jul 6 2017, 12:11 PM

@mat , how do you think of this one? Sorry that I'm a busy at the moment and cannot do the functional tests. In general, this patch looks good to me.

mat added inline comments.Jul 6 2017, 12:18 PM
Makefile
11–12

This is a bit awful, it should probably be changed into adding this to GH_TUPLE:

amutu:tensorflow-port:v${PORTVERSION}:patchs

and adding an:

EXTRA_PATCHES=   ${WRKSRC_patchs}/files/patch-WORKSPACE-${PORTVERSION} ${WRKSRC_patchs}/files/patch-tensorflow_workspace.bzl-${PORTVERSION}

The PORTVERSION could then probably be removed from the patch names.

@mat
Your idea is cool, this makes my maintain work easier, I have done the change, thanks!

amutu_amutu.com marked an inline comment as done.Jul 6 2017, 2:07 PM
mat edited edge metadata.Jul 6 2017, 2:13 PM

Also, both patches are not really that big, any reason they are not in the files directory as regular patches ?

In D11194#237955, @mat wrote:

Also, both patches are not really that big, any reason they are not in the files directory as regular patches ?

It is advised by @lwhsu, and we think it is bigger than other normal ports.

In D11194#237955, @mat wrote:

Also, both patches are not really that big, any reason they are not in the files directory as regular patches ?

It is advised by @lwhsu, and we think it is bigger than other normal ports.

lwhsu added a comment.Jul 6 2017, 2:25 PM
In D11194#237955, @mat wrote:

Also, both patches are not really that big, any reason they are not in the files directory as regular patches ?

Well, I somehow feel it's big but it seems we don't have a standard of "big" so I'm OK with both ways. Sorry for the noise.

The reason I suggested having version in the filename is for preventing filename conflict in user's /usr/ports/distfiles directory, in the future updating.

remove the extra patches and include all in patch files directory.

Hi @mat I am fine to include all the patches to this port too, and I have updated the diff to implement it.

If you are all too busy to test or commit this, please let me know, I will call for another committer who is interested in this on the mail list.

Thanks for all reviewers!

mat added a comment.Jul 12 2017, 12:42 PM

To get this committed, best would probably be to open a PR and submit the patch in bugzilla. This, here, is a code review tool, not really a bug reporting one :-)

lwhsu added a comment.Jul 12 2017, 1:05 PM
In D11194#239360, @mat wrote:

To get this committed, best would probably be to open a PR and submit the patch in bugzilla. This, here, is a code review tool, not really a bug reporting one :-)

Yes we do have one on bugzilla, with patch sync'd: http://bugs.freebsd.org/219609

Committed as rP447979

This differential can be closed now.

This revision is now accepted and ready to land.Aug 31 2017, 1:16 AM