Re: [PATCH] gnu: Add dlib.

[Marius Bakke <mbakke@fastmail.com>]

[-- Attachment #1: Type: text/plain, Size: 2683 bytes --]

Marius Bakke <mbakke@fastmail.com> writes:

> Leo Famulari <leo@famulari.name> writes:
>
>> On Wed, Aug 24, 2016 at 11:26:28AM +0100, Marius Bakke wrote:
>>> There are a couple of things going on in this thread:
>>> 
>>> 1. Segfault on x86_64. This seems to have been resolved simply by
>>> updating OpenBLAS. At least, I'm no longer able to reproduce it even
>>> with LAPACK in inputs. So, that should fix the Hydra x86_64 build.
>>> Can the OpenBLAS update be cherry-picked to master?
>>
>> I'd say it depends on whether the OpenBLAS users are building
>> successfully on core-updates, but unfortunately core-updates is
>> currently failing early in the bootstrap process [0]. Can you take a
>> look at `guix refresh -l dlib` and pick some important looking
>> applications to test with the updated OpenBLAS?
>
> I'm currently building the following openblas dependents: `libreoffice
> bamm python-biom-format clipper shogun armadillo julia` and will try to
> test BLAS functionality in some of them.

Shogun failed to build in this run. I don't have time to investigate
further, so picking the OpenBLAS update is not very appealing.

Instead I opted to disable the test that fails with lapack (and without,
on Hydra), since it's one specific openblas operation that is not unique
to dlib. I think it's an acceptable tradeoff, to give users the full
dlib functionality, and have the segfault "sort itself" when
core-updates lands in master.

>>> 2. i686 test failures. Updating OpenBLAS fixed 1/5 errors. The remaining
>>> four are reproducible on 32-bit Ubuntu, so they do not seem Guix
>>> related. Upstream has been notified.
>>> 
>>> 3. ARM failures. I don't have ARM hardware to test on, but I'm guessing
>>> it's similar to i686 (i.e. not directly Guix related).
>>
>> Maybe dlib is 64-bit only? If that's the case, we can disable it on
>> those architectures.
>
> According to the developer[0], these targets should be supported.
>
> 0: https://github.com/davisking/dlib/issues/197
>
> We could disable tests (at least the failing ones) on these platforms
> until this issue is resolved. The mips64el target on Hydra times out
> after 3600 seconds on one of the tests, but seems fine up to that point.
> Some of these tests are fairly CPU heavy, so the timeout may be too low.

Below is a patch which disables these tests (and the above segfault) for
19.1, rather than backporting the patches from dlib master branch.

One note about the patch: I could not figure out how to pass the list of
tests as arguments to `substitute*`, so currently it calls `substitute*`
for each of them. Any tips to prevent this?

It also no longer builds the main application twice for tests.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-gnu-dlib-Remove-unused-fftw-from-inputs.patch --]
[-- Type: text/x-patch, Size: 1280 bytes --]

From fe3b7409d32bbfd5c263d1d8f080b160ac676b84 Mon Sep 17 00:00:00 2001
From: Marius Bakke <mbakke@fastmail.com>
Date: Tue, 23 Aug 2016 21:17:14 +0100
Subject: [PATCH 1/2] gnu: dlib: Remove unused fftw from inputs.

* gnu/packages/machine-learning.scm (dlib)[inputs]: Remove fftw.
  (define-module): Don't include algebra.scm.
---
 gnu/packages/machine-learning.scm | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/gnu/packages/machine-learning.scm b/gnu/packages/machine-learning.scm
index 4332045..7669702 100644
--- a/gnu/packages/machine-learning.scm
+++ b/gnu/packages/machine-learning.scm
@@ -28,7 +28,6 @@
   #:use-module (guix build-system gnu)
   #:use-module (guix build-system r)
   #:use-module (gnu packages)
-  #:use-module (gnu packages algebra)
   #:use-module (gnu packages autotools)
   #:use-module (gnu packages boost)
   #:use-module (gnu packages compression)
@@ -515,8 +514,7 @@ single hidden layer, and for multinomial log-linear models.")
     (native-inputs
      `(("pkg-config" ,pkg-config)))
     (inputs
-     `(("fftw" ,fftw)
-       ("giflib" ,giflib)
+     `(("giflib" ,giflib)
        ;("lapack" ,lapack) XXX lapack here causes test failures in some setups.
        ("libjpeg" ,libjpeg)
        ("libpng" ,libpng)
-- 
2.9.3


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0002-gnu-dlib-Disable-failing-tests.patch --]
[-- Type: text/x-patch, Size: 2778 bytes --]

From 121895b2d7915b1f1ef0430921418783019393d0 Mon Sep 17 00:00:00 2001
From: Marius Bakke <mbakke@fastmail.com>
Date: Sat, 27 Aug 2016 17:23:58 +0100
Subject: [PATCH 2/2] gnu: dlib: Disable failing tests.

* gnu/packages/machine-learning.scm (dlib)[arguments]: Add
  'disable-failing-tests phase and prevent building dlib twice.
  [inputs]: Add lapack.
---
 gnu/packages/machine-learning.scm | 21 ++++++++++++++++-----
 1 file changed, 16 insertions(+), 5 deletions(-)

diff --git a/gnu/packages/machine-learning.scm b/gnu/packages/machine-learning.scm
index 7669702..35f1514 100644
--- a/gnu/packages/machine-learning.scm
+++ b/gnu/packages/machine-learning.scm
@@ -499,14 +499,25 @@ single hidden layer, and for multinomial log-linear models.")
              (substitute* "dlib/config.h"
                (("^//#define DLIB_DISABLE_ASSERTS") "#define DLIB_DISABLE_ASSERTS"))
              #t))
+         (add-after 'disable-asserts 'disable-failing-tests
+           ;; A number of tests are known to fail on 32-bit platforms in 19.1.
+           ;; See https://github.com/davisking/dlib/issues/197 for details.
+           (lambda _
+             (for-each
+              (lambda (test)
+                (substitute* "dlib/test/makefile"
+                  (((string-append "SRC \\+= " test "\\.cpp")) "")) #t)
+              (list "learning_to_track" "max_cost_assignment" ; armhf
+                    "optimization" "matrix2" "mpc" ; i686
+                    "empirical_map" ; may segfault with < openblas-0.2.18
+                    "object_detector")))) ; timeout on mips64el
          (replace 'check
            (lambda _
              ;; No test target, so we build and run the unit tests here.
-             (let ((test-dir (string-append "../dlib-" ,version "/dlib/test/build")))
-               (mkdir-p test-dir)
+             (let ((test-dir (string-append "../dlib-" ,version "/dlib/test")))
                (with-directory-excursion test-dir
-                 (and (zero? (system* "cmake" ".."))
-                      (zero? (system* "cmake" "--build" "." "--config" "Release"))
+                 (setenv "CXXFLAGS" "-std=gnu++11")
+                 (and (zero? (system* "make" "-j" (number->string (parallel-job-count))))
                       (zero? (system* "./dtest" "--runall")))))))
          (add-after 'install 'delete-static-library
            (lambda* (#:key outputs #:allow-other-keys)
@@ -515,7 +526,7 @@ single hidden layer, and for multinomial log-linear models.")
      `(("pkg-config" ,pkg-config)))
     (inputs
      `(("giflib" ,giflib)
-       ;("lapack" ,lapack) XXX lapack here causes test failures in some setups.
+       ("lapack" ,lapack)
        ("libjpeg" ,libjpeg)
        ("libpng" ,libpng)
        ("libx11" ,libx11)
-- 
2.9.3