diff mbox

R

Message ID 63b53a76-8343-38cf-ee2b-81ea517e50d9@uq.edu.au
State New
Headers show

Commit Message

Ben Woodcroft Sept. 8, 2016, 11:41 a.m. UTC
Hi,

I thought I'd respond to this old thread now that the openblas eigen 
error seems to be fixed. I'm interested in using openblas so that the 
(as yet not in master) WGCNA package works faster, among other things:

https://labs.genetics.ucla.edu/horvath/CoexpressionNetwork/Rpackages/WGCNA/faq.html


On 06/02/16 08:24, Andreas Enge wrote:
> On Fri, Feb 05, 2016 at 09:48:24PM +0100, Ricardo Wurmus wrote:
>> I would suggest to drop “--with-lapack”, too.  (I haven’t found the time
>> to try building without this flag just now.)  The manual says:
>> I don’t remember why I added it in the first place, so I think it’s best
>> to drop it, considering that the manual tells us only to do it if we
>> know what we want to achieve by adding it.
> I did, and it still builds on arm.

Shall I push the attached patch to core-updates? I tested it on x86_64. 
Or, would you mind testing it on arm please Andreas?

Thanks,
ben

Comments

Ricardo Wurmus Sept. 8, 2016, 1:02 p.m. UTC | #1
Ben Woodcroft <b.woodcroft@uq.edu.au> writes:

> Hi,
>
> I thought I'd respond to this old thread now that the openblas eigen 
> error seems to be fixed. I'm interested in using openblas so that the 
> (as yet not in master) WGCNA package works faster, among other things:
>
> https://labs.genetics.ucla.edu/horvath/CoexpressionNetwork/Rpackages/WGCNA/faq.html
>
>
> On 06/02/16 08:24, Andreas Enge wrote:
>> On Fri, Feb 05, 2016 at 09:48:24PM +0100, Ricardo Wurmus wrote:
>>> I would suggest to drop “--with-lapack”, too.  (I haven’t found the time
>>> to try building without this flag just now.)  The manual says:
>>> I don’t remember why I added it in the first place, so I think it’s best
>>> to drop it, considering that the manual tells us only to do it if we
>>> know what we want to achieve by adding it.
>> I did, and it still builds on arm.
>
> Shall I push the attached patch to core-updates? I tested it on x86_64. 
> Or, would you mind testing it on arm please Andreas?

The patch looks fine to me and I’m generally in favour of adding
OpenBLAS to R.  

I should note that the R admin manual[1] says that the correct configure
flag for OpenBLAS is

    --with-blas="-lopenblas"

I’m not sure if this makes a difference.

~~ Ricardo


[1]: https://cran.r-project.org/doc/manuals/r-release/R-admin.html#Goto-and-OpenBLAS
Pjotr Prins Sept. 8, 2016, 5:01 p.m. UTC | #2
OpenBLAS works fine for me - hit the same issue with Python before.

Pj.

On Thu, Sep 08, 2016 at 09:41:59PM +1000, Ben Woodcroft wrote:
> Hi,
> 
> I thought I'd respond to this old thread now that the openblas eigen
> error seems to be fixed. I'm interested in using openblas so that
> the (as yet not in master) WGCNA package works faster, among other
> things:
> 
> https://labs.genetics.ucla.edu/horvath/CoexpressionNetwork/Rpackages/WGCNA/faq.html
> 
> 
> On 06/02/16 08:24, Andreas Enge wrote:
> >On Fri, Feb 05, 2016 at 09:48:24PM +0100, Ricardo Wurmus wrote:
> >>I would suggest to drop “--with-lapack”, too.  (I haven’t found the time
> >>to try building without this flag just now.)  The manual says:
> >>I don’t remember why I added it in the first place, so I think it’s best
> >>to drop it, considering that the manual tells us only to do it if we
> >>know what we want to achieve by adding it.
> >I did, and it still builds on arm.
> 
> Shall I push the attached patch to core-updates? I tested it on
> x86_64. Or, would you mind testing it on arm please Andreas?
> 
> Thanks,
> ben

> From 012c013661c739b8d90be8bcbfce13349dcaee89 Mon Sep 17 00:00:00 2001
> From: Ben J Woodcroft <donttrustben@gmail.com>
> Date: Thu, 8 Sep 2016 21:14:12 +1000
> Subject: [PATCH] gnu: r: Add openblas input.
> 
> * gnu/packages/statistics.scm (r)[inputs]: Add openblas.
> [arguments]: Adapt configure flags.
> ---
>  gnu/packages/statistics.scm | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/gnu/packages/statistics.scm b/gnu/packages/statistics.scm
> index 990f2fc..e34bcec 100644
> --- a/gnu/packages/statistics.scm
> +++ b/gnu/packages/statistics.scm
> @@ -136,6 +136,7 @@ be output in text, PostScript, PDF or HTML.")
>            (lambda _ (zero? (system* "make" "install-info")))))
>         #:configure-flags
>         '("--with-cairo"
> +         "--with-blas=openblas"
>           "--with-libpng"
>           "--with-jpeglib"
>           "--with-libtiff"
> @@ -170,6 +171,7 @@ be output in text, PostScript, PDF or HTML.")
>         ("pango" ,pango)
>         ("curl" ,curl)
>         ("tzdata" ,tzdata)
> +       ("openblas" ,openblas)
>         ("gfortran" ,gfortran)
>         ("icu4c" ,icu4c)
>         ("libjpeg" ,libjpeg)
> -- 
> 2.9.2
> 


--
Ben Woodcroft Sept. 9, 2016, 12:07 p.m. UTC | #3
On 08/09/16 23:02, Ricardo Wurmus wrote:
> Ben Woodcroft <b.woodcroft@uq.edu.au> writes:
>
>> Hi,
>>
>> I thought I'd respond to this old thread now that the openblas eigen
>> error seems to be fixed. I'm interested in using openblas so that the
>> (as yet not in master) WGCNA package works faster, among other things:
>>
>> https://labs.genetics.ucla.edu/horvath/CoexpressionNetwork/Rpackages/WGCNA/faq.html
>>
>>
>> On 06/02/16 08:24, Andreas Enge wrote:
>>> On Fri, Feb 05, 2016 at 09:48:24PM +0100, Ricardo Wurmus wrote:
>>>> I would suggest to drop “--with-lapack”, too.  (I haven’t found the time
>>>> to try building without this flag just now.)  The manual says:
>>>> I don’t remember why I added it in the first place, so I think it’s best
>>>> to drop it, considering that the manual tells us only to do it if we
>>>> know what we want to achieve by adding it.
>>> I did, and it still builds on arm.
>> Shall I push the attached patch to core-updates? I tested it on x86_64.
>> Or, would you mind testing it on arm please Andreas?
> The patch looks fine to me and I’m generally in favour of adding
> OpenBLAS to R.
>
> I should note that the R admin manual[1] says that the correct configure
> flag for OpenBLAS is
>
>      --with-blas="-lopenblas"
>
> I’m not sure if this makes a difference.

Thanks. I changed it to that minus the double quotes, and updated to 
3.3.1, then pushed to core-updated. We'll see how things go.
ta
diff mbox

Patch

From 012c013661c739b8d90be8bcbfce13349dcaee89 Mon Sep 17 00:00:00 2001
From: Ben J Woodcroft <donttrustben@gmail.com>
Date: Thu, 8 Sep 2016 21:14:12 +1000
Subject: [PATCH] gnu: r: Add openblas input.

* gnu/packages/statistics.scm (r)[inputs]: Add openblas.
[arguments]: Adapt configure flags.
---
 gnu/packages/statistics.scm | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/gnu/packages/statistics.scm b/gnu/packages/statistics.scm
index 990f2fc..e34bcec 100644
--- a/gnu/packages/statistics.scm
+++ b/gnu/packages/statistics.scm
@@ -136,6 +136,7 @@  be output in text, PostScript, PDF or HTML.")
           (lambda _ (zero? (system* "make" "install-info")))))
        #:configure-flags
        '("--with-cairo"
+         "--with-blas=openblas"
          "--with-libpng"
          "--with-jpeglib"
          "--with-libtiff"
@@ -170,6 +171,7 @@  be output in text, PostScript, PDF or HTML.")
        ("pango" ,pango)
        ("curl" ,curl)
        ("tzdata" ,tzdata)
+       ("openblas" ,openblas)
        ("gfortran" ,gfortran)
        ("icu4c" ,icu4c)
        ("libjpeg" ,libjpeg)
-- 
2.9.2