diff mbox

gnu: Add dlib.

Message ID 87mvjuz5ae.fsf@ike.i-did-not-set--mail-host-address--so-tickle-me
State New
Headers show

Commit Message

Marius Bakke Aug. 30, 2016, 2:43 p.m. UTC 
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.

Comments

Leo Famulari Aug. 31, 2016, 7:09 p.m. UTC | #1 
On Tue, Aug 30, 2016 at 03:43:05PM +0100, Marius Bakke wrote:
> 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.

Okay, this sounds fine to me.

> 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?

Not from me — Calling all seasoned Schemers to thread :) If nobody
replies I will say this solution is fine.

Changing the subject, you could disable the tests per-architecture. Look
for uses of current-target-system and current-system for usage examples.
But this is not absolutely required, IMO.

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

Thank you :)
diff mbox

Patch

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