Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

module support #10

Closed
uhoefel opened this issue Feb 12, 2022 · 6 comments
Closed

module support #10

uhoefel opened this issue Feb 12, 2022 · 6 comments

Comments

@uhoefel
Copy link
Contributor

uhoefel commented Feb 12, 2022

Currently I get the following error trying to use both LAPACK and BLAS in my project:

image

The relevant piece from my module-info:

    requires lapack;
    requires arpack.combined.all;
    requires blas;

(ARPACK is necessary for the intW type needed e.g. for dpotf2)

my pom.xml:

<dependency>
  <groupId>dev.ludovic.netlib</groupId>
  <artifactId>lapack</artifactId>
  <version>2.2.1</version>
</dependency>
<dependency>
  <groupId>dev.ludovic.netlib</groupId>
  <artifactId>arpack</artifactId>
  <version>2.2.1</version>
</dependency>
<dependency>
  <groupId>dev.ludovic.netlib</groupId>
  <artifactId>blas</artifactId>
  <version>2.2.1</version>
</dependency>

I think this can be fixed by adding module support, which would also remove the warning coming with the packages at the moment:
image

@luhenry
Copy link
Owner

luhenry commented Feb 19, 2022

Hi @uhoefel. As the error indicates, dev.ludovic.netlib is currently using a split package approach: the dev.ludovic.netlib package is defined in multiple artifacts. Unfortunately, that seems to be incompatible with modules...

Changing that API would imply an API breakage, as it would need to move dev.ludovic.netlib.BLAS, dev.ludovic.netlib.LAPACK, dev.ludovic.netlib.ARPACK, and all the other dev.ludovic.netlib.* classes into their respective dev.ludovic.netlib.blas, dev.ludovic.netlib.lapack, and dev.ludovic.netlib.arpack packages.

Which version of the JDK is that currently blocking you on?

@uhoefel
Copy link
Contributor Author

uhoefel commented Feb 19, 2022

I use java 17:

openjdk 17.0.1 2021-10-19
OpenJDK Runtime Environment GraalVM CE 21.3.0 (build 17.0.1+12-jvmci-21.3-b05)
OpenJDK 64-Bit Server VM GraalVM CE 21.3.0 (build 17.0.1+12-jvmci-21.3-b05, mixed mode, sharing)

and I use modules in my project.

@jonathanschilling
Copy link
Contributor

Hi @luhenry,

I am working with @uhoefel and strongly support to migrate our codebase
(which currently still relies on the ancient fommil/netlib-java) to this project.
It is the right time now to fix this issue, since this project is not so widely used yet.

It would be awesome if there would be a Java package providing BLAS and LAPACK using the Java Vector API
and I think this project could be the one.
However, if this issue is not fixed before more people start using this project,
this might significantly limit usability in the future when the Java module system
will have become more mandatory to use.

What do you think?

@luhenry
Copy link
Owner

luhenry commented May 12, 2022

@jonathanschilling I agree with your take. I will try to give it some time in the coming weeks. I also welcome any contribution, and would be very happy to integrate your changes to support module. I can then take care of updating its usage in Spark to match the change of API.

Also, this package has a BLAS implementation using the Vector API at https://github.com/luhenry/netlib/blob/master/blas/src/main/java/dev/ludovic/netlib/blas/VectorBLAS.java. There isn't any LAPACK implementation since the surface is so much larger.

The major performance blocker while using the Vector API was limitations in the register allocator which would lead to register spilling leading to not-optimal performance (max ~16Gflops vs ~22Gflops with OpenBLAS). Without this register spilling, the GEMM would be on-par performance wise between the Vector API and OpenBLAS/Intel MKL.

@luhenry
Copy link
Owner

luhenry commented Jul 15, 2022

This has been fixed with https://github.com/luhenry/netlib/releases/tag/v3.0.0

@luhenry luhenry closed this as completed Jul 15, 2022
@jonathanschilling
Copy link
Contributor

Thanks a lot for resolving this issue! 👍

srowen pushed a commit to apache/spark that referenced this issue Sep 2, 2022
…3.0.2 & breeze from 2.0 to 2.1.0

### What changes were proposed in this pull request?
The pr aim to
> A.Upgrade dev.ludovic.netlib from 2.2.1 to 3.0.2
> B.Upgrade breeze from 2.0 to 2.1.0

FYI
<img width="657" alt="image" src="https://user-images.githubusercontent.com/15246973/187114740-2af5f7fa-4297-410c-8276-d9dfba8af707.png">

### Why are the changes needed?
A.dev.ludovic.netlib
> This version contain bug fix:
> luhenry/netlib#12
> luhenry/netlib#10

> The changes between 2.2.1 and 3.0.2 as follows:
> luhenry/netlib@v2.2.1...v3.0.2

B.breeze
> The changes between 2.0 and 2.1.0 as follows:
> scalanlp/breeze@releases/v2.0...releases/v2.1.0

### Does this PR introduce _any_ user-facing change?
No.

### How was this patch tested?
Pass GA & exist UT.

Closes #37700 from panbingkun/upgrade_netlib.

Authored-by: panbingkun <pbk1982@gmail.com>
Signed-off-by: Sean Owen <srowen@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants