Bug#823140: RFS: caffe/1.0.0~rc3-1 -- a deep learning framework [ITP]
On 17/05/16 15:42, lumin wrote:
Thank you for this careful and thorough review!
>
2) why the python3 support is disabled? note: this will require a trip in new queue, so if possible
I would prefer a python3-only build
Same here. Since the build system leaves you the choice, please consider
packaging the Python 3 version. Python 2 has an expiration date anyway.
As said in Debian's python policy I'm aware of it. Let me
clarify this point here. one of the build dependencies of
python-caffe-* is python*-protobuf, and python3-protobuf
is not possible for current protobuf version in Debian.
In a word, build dependencies not satisfied for python3-caffe-*.
You're right, I think I saw a post on d-devel of someone attempting to
package protobuf for Py3. My bad.
* On a different note, caffe seems to support building the documentation
from source with doxygen. Please consider packaging it in a separate
binary package (caffe-doc). The content of examples/* and models/*
might also fit in this -doc package.
Yeah but I'm thinking about the latex(pdf) document generation,
I don't know should I add Build-Indep on texlive or is there
a "core" package to complete doxygen latex compiling.
doxygen-latex [1]?
[1] https://packages.debian.org/sid/doxygen-latex
I'll build caffe-doc package from "caffe" source, and also recommend
"caffe-doc" package in packages from "caffe-contrib".
We usually put -doc in Suggests for the -dev or -bin package, depending
on what the docs cover (API, user guide...).
* You should consider adding a packaging testsuite. You could start with
a script running part or all the examples shipped in the -doc package,
which would verify that the packaged software run as intended.
Caffe has complete gtest testing routings. According to my
experience of using Caffe, it should be working
if it passed the dh_auto_test.
Just to clarify, I meant this sort of packaging testsuite [2].
[2] https://people.debian.org/~mpitt/autopkgtest/README.package-tests.html
What happens during dh_auto_test is completely different. The packaging
testsuite is for CI purposes.
Adding more test is easy, and I think some more testsuite
from me will benifit other maintainers since they can learn
from it how to test this software by hand.
I'll do this.
As said earlier, the minimum is to test what you **install**. Making
sure the packaged examples can be built and run from the packaged
library is an example of such test case.
* You should consider adding some upstream metadata as described in [1].
I am sure the Caffe guys would appreciate that we appropriately forward
the citation information displayed on their README.
I didn't know Debian has such a cool thing. Will do this.
Later, you'll be able to watch the results on the d-science sentinel
for the machine-learning task [3]. The citation will appear at the
bottom of the description, like in other packages.
[3] http://blends.debian.org/science/tasks/machine-learning
* What is the purpose of keeping unapplied patches in d/patches?
Uh I forgot to remove them ... Will fix it.
* Missing uversionmangling in d/watch for appropriate tracking of
release candidate releases.
Since uscan(1) describes uversionmangle I think I can manage it.
* Consider using libblas-dev | libblas.so as Build-Depends instead of
libopenblas-dev. Not everyone is using openblas as its prefered blas
implementation and upstream does not force to use this specific vendor
(do they?), so no reason to force it here either.
No, I don't agree using libblas-dev as build-dep. Caffe only supports
3 BLAS backends: atlas, openblas and Intel MKL. Besides, openblas surpasses
basic BLAS by a large margin on performance, we should not switch
build-dep from openblas/atlas back to basic blas for a computational
intensive application, which would be awkward.
There is a discussion thread about which BLAS to use for the debian package.
Some caffe user believes that, it doesn't matter what BLAS the
maintainer choose, any high-performance one will do.
So I chose openblas after an investigation.
see
https://github.com/BVLC/caffe/issues/2601
You have heard of alternatives [4], haven't you?
[4] https://wiki.debian.org/DebianAlternatives
So, if you b-dep on libopenblas-dev, you will force a linkage on one
specific vendor for blas, and bypass the alternatives concept. This is
what you noticed with `readelf -d` in the issue.
Instead, you should be using libblas-dev | libblas.so, which will
provide a virtual dependency (libblas.so.3) which can be satisfied by
either vendors of blas. See the result with ArrayFire [5].
[5] https://packages.debian.org/sid/libarrayfire-cpu3
* For the future, seems like additional caffe-tools and octave-caffe
binary packages could be created from the content generated by the
matlab/ and tools/ leaves of the source tree. I guess that's what is
acknowledged in debian/TODO.
ELFs under tools/ are shipped in package "caffe-{cpu,cuda}". I prefer this
scheme rather than "caffe-cpu(metapackage) + caffe-tools(elfs)".
BTW I'd like to create a metapackage "caffe-cpu-all".
For octave-caffe, I haven't find the way to pass the build with octave
yet. I'd like to postpone this binary package to future uploads.
OpenCL support from caffe upstream is still *experimental*, but
it will come one day, hence it was added to TODO. caffe-doc
and octave-caffe are as said above.
Indeed, not so important at this point. These can be added later if
need arises.
Thanks for working on this.
Ghis
Reply to: