Fixed BLAS tests for external BLAS library

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

Fixed BLAS tests for external BLAS library

Simon Guest
We build R with dynamically linked BLAS and LAPACK libraries, in order
to use the AMD Core Math Library (ACML) multi-threaded implementation
of these routines on our 64 core servers.  This is great, and our
users really appreciate it.

However, when building like this, make check fails on the reg-BLAS.R
test.  The reason for this is that the expected test output is checked
using identical.  By changing all uses of identical in this file to
all.equal, the tests pass.

Specifically, I run this command before make check:

$ sed -i -e 's/identical/all.equal/g' tests/reg-BLAS.R

I suggest that the test is fixed like this in the R source code.

Here is the configure command I use, on CentOS 7:
$ ./configure --with-system-zlib --with-system-bzlib --with-system-pcre \
    --with-blas \
    --with-lapack \
    --with-tcl-config=/usr/lib64/tclConfig.sh \
    --with-tk-config=/usr/lib64/tkConfig.sh \
    --enable-R-shlib \
    --enable-prebuilt-html

cheers,
Simon

______________________________________________
[hidden email] mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel
Reply | Threaded
Open this post in threaded view
|

Re: Fixed BLAS tests for external BLAS library

Simon Guest-2
Hi Tomas,

Thanks for your reply.

I find your response curious, however.  Surely the identical() test is
simply incorrect when catering for possibly different BLAS
implementations?  Or is it the case that conformant BLAS implementations
all produce bit-identical results, which seems unlikely?  (Sorry, I am
unfamiliar with the BLAS spec.)  Although whatever the answer to this
theoretical question, the CentOS 7 external BLAS library evidently doesn't
produce bit-identical results.

If you don't agree that replacing identical() with all.equal() is clearly
the right thing to do, as demonstrated by the CentOS 7 external BLAS
library failing the test, then I think I will give up now trying to help
improve the R sources.  I simply can't justify to my client more time spent
on making this work, when we already have a local solution (which I hoped
others would be able to benefit from).  Ah well.

cheers,
Simon

On 5 January 2018 at 00:07, Tomas Kalibera <[hidden email]> wrote:

> Hi Simon,
>
> we'd need more information to consider this - particularly which
> expression gives an imprecise result with ACML and what are the computed
> values, differences. It is not common for optimized BLAS implementations to
> fail reg-BLAS.R tests, but it is common for them to report numerical
> differences in tests of various recommended packages where more complicated
> computations are done (e.g. nlme), on various platforms.
>
> Best
> Tomas
>
>
> On 12/18/2017 08:56 PM, Simon Guest wrote:
>
>> We build R with dynamically linked BLAS and LAPACK libraries, in order
>> to use the AMD Core Math Library (ACML) multi-threaded implementation
>> of these routines on our 64 core servers.  This is great, and our
>> users really appreciate it.
>>
>> However, when building like this, make check fails on the reg-BLAS.R
>> test.  The reason for this is that the expected test output is checked
>> using identical.  By changing all uses of identical in this file to
>> all.equal, the tests pass.
>>
>> Specifically, I run this command before make check:
>>
>> $ sed -i -e 's/identical/all.equal/g' tests/reg-BLAS.R
>>
>> I suggest that the test is fixed like this in the R source code.
>>
>> Here is the configure command I use, on CentOS 7:
>> $ ./configure --with-system-zlib --with-system-bzlib --with-system-pcre \
>>      --with-blas \
>>      --with-lapack \
>>      --with-tcl-config=/usr/lib64/tclConfig.sh \
>>      --with-tk-config=/usr/lib64/tkConfig.sh \
>>      --enable-R-shlib \
>>      --enable-prebuilt-html
>>
>> cheers,
>> Simon
>>
>> ______________________________________________
>> [hidden email] mailing list
>> https://stat.ethz.ch/mailman/listinfo/r-devel
>>
>
>
>

        [[alternative HTML version deleted]]

______________________________________________
[hidden email] mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel
Reply | Threaded
Open this post in threaded view
|

Re: Fixed BLAS tests for external BLAS library

Tomas Kalibera

In practical terms, failing tests are not preventing anyone from using
an optimized BLAS/LAPACK implementation they trust. Building R with
dynamically linked BLAS on Unix is supported, documented and easy for
anyone who builds R from source. It is also how Debian/Ubuntu R packages
are built by default, so R uses whichever BLAS is installed in the
system and the user does not have to build from source. There is no
reason why not to do the same thing with another optimized BLAS on
another OS/distribution.

You may be right that reg-BLAS is too strict (it is testing matrix
products, expecting equivalence to naive three-loop algorithm, just part
of it really uses BLAS). I just wanted a concrete example to think about
as I can't repeat it (e.g. it passes with openblas), but maybe someone
else will be able to repeat and possibly adjust.

Tomas

On 01/04/2018 09:23 PM, Simon Guest wrote:

> Hi Tomas,
>
> Thanks for your reply.
>
> I find your response curious, however.  Surely the identical() test is
> simply incorrect when catering for possibly different BLAS
> implementations?  Or is it the case that conformant BLAS
> implementations all produce bit-identical results, which seems
> unlikely?  (Sorry, I am unfamiliar with the BLAS spec.)  Although
> whatever the answer to this theoretical question, the CentOS 7
> external BLAS library evidently doesn't produce bit-identical results.
>
> If you don't agree that replacing identical() with all.equal() is
> clearly the right thing to do, as demonstrated by the CentOS 7
> external BLAS library failing the test, then I think I will give up
> now trying to help improve the R sources.  I simply can't justify to
> my client more time spent on making this work, when we already have a
> local solution (which I hoped others would be able to benefit from). 
> Ah well.
>
> cheers,
> Simon
>
> On 5 January 2018 at 00:07, Tomas Kalibera <[hidden email]
> <mailto:[hidden email]>> wrote:
>
>     Hi Simon,
>
>     we'd need more information to consider this - particularly which
>     expression gives an imprecise result with ACML and what are the
>     computed values, differences. It is not common for optimized BLAS
>     implementations to fail reg-BLAS.R tests, but it is common for
>     them to report numerical differences in tests of various
>     recommended packages where more complicated computations are done
>     (e.g. nlme), on various platforms.
>
>     Best
>     Tomas
>
>
>     On 12/18/2017 08:56 PM, Simon Guest wrote:
>
>         We build R with dynamically linked BLAS and LAPACK libraries,
>         in order
>         to use the AMD Core Math Library (ACML) multi-threaded
>         implementation
>         of these routines on our 64 core servers.  This is great, and our
>         users really appreciate it.
>
>         However, when building like this, make check fails on the
>         reg-BLAS.R
>         test.  The reason for this is that the expected test output is
>         checked
>         using identical.  By changing all uses of identical in this
>         file to
>         all.equal, the tests pass.
>
>         Specifically, I run this command before make check:
>
>         $ sed -i -e 's/identical/all.equal/g' tests/reg-BLAS.R
>
>         I suggest that the test is fixed like this in the R source code.
>
>         Here is the configure command I use, on CentOS 7:
>         $ ./configure --with-system-zlib --with-system-bzlib
>         --with-system-pcre \
>              --with-blas \
>              --with-lapack \
>              --with-tcl-config=/usr/lib64/tclConfig.sh \
>              --with-tk-config=/usr/lib64/tkConfig.sh \
>              --enable-R-shlib \
>              --enable-prebuilt-html
>
>         cheers,
>         Simon
>
>         ______________________________________________
>         [hidden email] <mailto:[hidden email]> mailing list
>         https://stat.ethz.ch/mailman/listinfo/r-devel
>         <https://stat.ethz.ch/mailman/listinfo/r-devel>
>
>
>
>


        [[alternative HTML version deleted]]

______________________________________________
[hidden email] mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel
Reply | Threaded
Open this post in threaded view
|

Re: Fixed BLAS tests for external BLAS library

Martin Maechler
>>>>> Tomas Kalibera <[hidden email]>
>>>>>     on Fri, 5 Jan 2018 00:41:47 +0100 writes:

    > In practical terms, failing tests are not preventing anyone from using
    > an optimized BLAS/LAPACK implementation they trust. Building R with
    > dynamically linked BLAS on Unix is supported, documented and easy for
    > anyone who builds R from source. It is also how Debian/Ubuntu R packages
    > are built by default, so R uses whichever BLAS is installed in the
    > system and the user does not have to build from source. There is no
    > reason why not to do the same thing with another optimized BLAS on
    > another OS/distribution.

    > You may be right that reg-BLAS is too strict (it is testing matrix
    > products, expecting equivalence to naive three-loop algorithm, just part
    > of it really uses BLAS). I just wanted a concrete example to think about
    > as I can't repeat it (e.g. it passes with openblas), but maybe someone
    > else will be able to repeat and possibly adjust.

    > Tomas

Yes, indeed!  I strongly agree with Thomas:  This is about
serious quality assurance of an important part of R,
and replacing all identical() checks there with all.equal()
-- which has a default tolerance of allowing __HALF__ of the
   precision being lost  !! -- in the way you, Simon, proposed,
is definitely basically destroying the QC/QA  we have in place there.

As Tomas said, *some* of the checks possibly should be done
all.equal, but with a very a small tolerance; however other
checks should not allow a tolerance, e.g., all the arithmetic involving
very small integer valued numbers should definitely be exact.

That's why Tomas' (private!) reply, asking for specific details
is 100% appropriate, indeed.

With R we have had a philosophy of trying hard to be correct
first, and fast second... and indeed the last 20 years have
shown many cases where R's use (and checks) actually have
reveiled not only inaccuracies but sometimes also bugs in
LAPACK/BLAS implementations where it sometimes seems, some are
only interested in speed, rather than correctness.

Martin Maechler
ETH Zurich

    > On 01/04/2018 09:23 PM, Simon Guest wrote:
    >> Hi Tomas,
    >>
    >> Thanks for your reply.
    >>
    >> I find your response curious, however.  Surely the identical() test is
    >> simply incorrect when catering for possibly different BLAS
    >> implementations?  Or is it the case that conformant BLAS
    >> implementations all produce bit-identical results, which seems
    >> unlikely?  (Sorry, I am unfamiliar with the BLAS spec.)  Although
    >> whatever the answer to this theoretical question, the CentOS 7
    >> external BLAS library evidently doesn't produce bit-identical results.
    >>
    >> If you don't agree that replacing identical() with all.equal() is
    >> clearly the right thing to do, as demonstrated by the CentOS 7
    >> external BLAS library failing the test, then I think I will give up
    >> now trying to help improve the R sources.  I simply can't justify to
    >> my client more time spent on making this work, when we already have a
    >> local solution (which I hoped others would be able to benefit from). 
    >> Ah well.
    >>
    >> cheers,
    >> Simon
    >>
    >> On 5 January 2018 at 00:07, Tomas Kalibera <[hidden email]
    >> <mailto:[hidden email]>> wrote:
    >>
    >> Hi Simon,
    >>
    >> we'd need more information to consider this - particularly which
    >> expression gives an imprecise result with ACML and what are the
    >> computed values, differences. It is not common for optimized BLAS
    >> implementations to fail reg-BLAS.R tests, but it is common for
    >> them to report numerical differences in tests of various
    >> recommended packages where more complicated computations are done
    >> (e.g. nlme), on various platforms.
    >>
    >> Best
    >> Tomas
    >>
    >>
    >> On 12/18/2017 08:56 PM, Simon Guest wrote:
    >>
    >> We build R with dynamically linked BLAS and LAPACK libraries,
    >> in order
    >> to use the AMD Core Math Library (ACML) multi-threaded
    >> implementation
    >> of these routines on our 64 core servers.  This is great, and our
    >> users really appreciate it.
    >>
    >> However, when building like this, make check fails on the
    >> reg-BLAS.R
    >> test.  The reason for this is that the expected test output is
    >> checked
    >> using identical.  By changing all uses of identical in this
    >> file to
    >> all.equal, the tests pass.
    >>
    >> Specifically, I run this command before make check:
    >>
    >> $ sed -i -e 's/identical/all.equal/g' tests/reg-BLAS.R
    >>
    >> I suggest that the test is fixed like this in the R source code.
    >>
    >> Here is the configure command I use, on CentOS 7:
    >> $ ./configure --with-system-zlib --with-system-bzlib
    >> --with-system-pcre \
    >>      --with-blas \
    >>      --with-lapack \
    >>      --with-tcl-config=/usr/lib64/tclConfig.sh \
    >>      --with-tk-config=/usr/lib64/tkConfig.sh \
    >>      --enable-R-shlib \
    >>      --enable-prebuilt-html
    >>
    >> cheers,
    >> Simon
    >>
    >> ______________________________________________
    >> [hidden email] <mailto:[hidden email]> mailing list
    >> https://stat.ethz.ch/mailman/listinfo/r-devel
    >> <https://stat.ethz.ch/mailman/listinfo/r-devel>
    >>
    >>
    >>
    >>


    > [[alternative HTML version deleted]]

    > ______________________________________________
    > [hidden email] mailing list
    > https://stat.ethz.ch/mailman/listinfo/r-devel

______________________________________________
[hidden email] mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel
Reply | Threaded
Open this post in threaded view
|

Re: Fixed BLAS tests for external BLAS library

Simon Guest-2
Hi Martin and Tomas,

Thanks for your reasoned replies.  It seems that improving this is going to
take more effort in pinning down exactly what is appropriate than I
anticipated.

Sorry if the intention was to keep the initial discussion of this off the
list, I didn't mean to cause offence by copying my reply to the list.

I think I have to acknowledge that I am insufficiently familiar with R to
be able to do what you require in a reasonable time, so I will have to
leave this to someone else to pursue if they are so inclined.  (I am an
infrastructure engineer rather than an R programmer.) For now, then, I will
continue as before, and locally patch that test to pass with the external
BLAS library on CentOS 7.

cheers,
Simon

On 5 January 2018 at 21:56, Martin Maechler <[hidden email]>
wrote:

> >>>>> Tomas Kalibera <[hidden email]>
> >>>>>     on Fri, 5 Jan 2018 00:41:47 +0100 writes:
>
>     > In practical terms, failing tests are not preventing anyone from
> using
>     > an optimized BLAS/LAPACK implementation they trust. Building R with
>     > dynamically linked BLAS on Unix is supported, documented and easy for
>     > anyone who builds R from source. It is also how Debian/Ubuntu R
> packages
>     > are built by default, so R uses whichever BLAS is installed in the
>     > system and the user does not have to build from source. There is no
>     > reason why not to do the same thing with another optimized BLAS on
>     > another OS/distribution.
>
>     > You may be right that reg-BLAS is too strict (it is testing matrix
>     > products, expecting equivalence to naive three-loop algorithm, just
> part
>     > of it really uses BLAS). I just wanted a concrete example to think
> about
>     > as I can't repeat it (e.g. it passes with openblas), but maybe
> someone
>     > else will be able to repeat and possibly adjust.
>
>     > Tomas
>
> Yes, indeed!  I strongly agree with Thomas:  This is about
> serious quality assurance of an important part of R,
> and replacing all identical() checks there with all.equal()
> -- which has a default tolerance of allowing __HALF__ of the
>    precision being lost  !! -- in the way you, Simon, proposed,
> is definitely basically destroying the QC/QA  we have in place there.
>
> As Tomas said, *some* of the checks possibly should be done
> all.equal, but with a very a small tolerance; however other
> checks should not allow a tolerance, e.g., all the arithmetic involving
> very small integer valued numbers should definitely be exact.
>
> That's why Tomas' (private!) reply, asking for specific details
> is 100% appropriate, indeed.
>
> With R we have had a philosophy of trying hard to be correct
> first, and fast second... and indeed the last 20 years have
> shown many cases where R's use (and checks) actually have
> reveiled not only inaccuracies but sometimes also bugs in
> LAPACK/BLAS implementations where it sometimes seems, some are
> only interested in speed, rather than correctness.
>
> Martin Maechler
> ETH Zurich
>
>     > On 01/04/2018 09:23 PM, Simon Guest wrote:
>     >> Hi Tomas,
>     >>
>     >> Thanks for your reply.
>     >>
>     >> I find your response curious, however.  Surely the identical() test
> is
>     >> simply incorrect when catering for possibly different BLAS
>     >> implementations?  Or is it the case that conformant BLAS
>     >> implementations all produce bit-identical results, which seems
>     >> unlikely?  (Sorry, I am unfamiliar with the BLAS spec.)  Although
>     >> whatever the answer to this theoretical question, the CentOS 7
>     >> external BLAS library evidently doesn't produce bit-identical
> results.
>     >>
>     >> If you don't agree that replacing identical() with all.equal() is
>     >> clearly the right thing to do, as demonstrated by the CentOS 7
>     >> external BLAS library failing the test, then I think I will give up
>     >> now trying to help improve the R sources.  I simply can't justify to
>     >> my client more time spent on making this work, when we already have
> a
>     >> local solution (which I hoped others would be able to benefit
> from).
>     >> Ah well.
>     >>
>     >> cheers,
>     >> Simon
>     >>
>     >> On 5 January 2018 at 00:07, Tomas Kalibera <
> [hidden email]
>     >> <mailto:[hidden email]>> wrote:
>     >>
>     >> Hi Simon,
>     >>
>     >> we'd need more information to consider this - particularly which
>     >> expression gives an imprecise result with ACML and what are the
>     >> computed values, differences. It is not common for optimized BLAS
>     >> implementations to fail reg-BLAS.R tests, but it is common for
>     >> them to report numerical differences in tests of various
>     >> recommended packages where more complicated computations are done
>     >> (e.g. nlme), on various platforms.
>     >>
>     >> Best
>     >> Tomas
>     >>
>     >>
>     >> On 12/18/2017 08:56 PM, Simon Guest wrote:
>     >>
>     >> We build R with dynamically linked BLAS and LAPACK libraries,
>     >> in order
>     >> to use the AMD Core Math Library (ACML) multi-threaded
>     >> implementation
>     >> of these routines on our 64 core servers.  This is great, and our
>     >> users really appreciate it.
>     >>
>     >> However, when building like this, make check fails on the
>     >> reg-BLAS.R
>     >> test.  The reason for this is that the expected test output is
>     >> checked
>     >> using identical.  By changing all uses of identical in this
>     >> file to
>     >> all.equal, the tests pass.
>     >>
>     >> Specifically, I run this command before make check:
>     >>
>     >> $ sed -i -e 's/identical/all.equal/g' tests/reg-BLAS.R
>     >>
>     >> I suggest that the test is fixed like this in the R source code.
>     >>
>     >> Here is the configure command I use, on CentOS 7:
>     >> $ ./configure --with-system-zlib --with-system-bzlib
>     >> --with-system-pcre \
>     >>      --with-blas \
>     >>      --with-lapack \
>     >>      --with-tcl-config=/usr/lib64/tclConfig.sh \
>     >>      --with-tk-config=/usr/lib64/tkConfig.sh \
>     >>      --enable-R-shlib \
>     >>      --enable-prebuilt-html
>     >>
>     >> cheers,
>     >> Simon
>     >>
>     >> ______________________________________________
>     >> [hidden email] <mailto:[hidden email]> mailing list
>     >> https://stat.ethz.ch/mailman/listinfo/r-devel
>     >> <https://stat.ethz.ch/mailman/listinfo/r-devel>
>     >>
>     >>
>     >>
>     >>
>
>
>     > [[alternative HTML version deleted]]
>
>     > ______________________________________________
>     > [hidden email] mailing list
>     > https://stat.ethz.ch/mailman/listinfo/r-devel
>
> ______________________________________________
> [hidden email] mailing list
> https://stat.ethz.ch/mailman/listinfo/r-devel
>

        [[alternative HTML version deleted]]

______________________________________________
[hidden email] mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel