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

Adaptations for Panua Pardiso #75

Merged
merged 17 commits into from
Mar 1, 2024
Merged

Conversation

j-fu
Copy link
Collaborator

@j-fu j-fu commented Apr 9, 2021

Hi, trying to add support for Pardiso7 here. AFAIK, it has no breaking API changes wrt. 6.x.

However runtests bailed out in the indefinite hermitian case as Pardiso doesn't report an error in this case. Nevetheless, it does not solve, so I compare hashes of X in order to catch this case. May be this is not the smartest way, but works.

Will discuss this with upstream.

@KristofferC
Copy link
Member

However runtests bailed out in the indefinite hermitian case as Pardiso doesn't report an error in this case.

Is there no error code at all or something else that can be inspect for this?

@j-fu
Copy link
Collaborator Author

j-fu commented Apr 9, 2021

Just contacted upstream wrt to this. I assume it is a bug in Pardiso (I checked: Err[] is zero), as it knows to do nothing in this case. EDIT: We may wait for a response...

@j-fu
Copy link
Collaborator Author

j-fu commented Apr 9, 2021

The PR hashes X before and after, and figures this out (so runtests works with on my system), but this is not optimal.

@j-fu
Copy link
Collaborator Author

j-fu commented Apr 22, 2021

Hi, I contacted upstream, contact was acknowledged, but I did not get a response so far, they must be really busy. I propose to move on with the PR and keep the hashing hack in order to get Pardiso7 functionality, the more that new users won't be able to get Pardiso6 anymore.

  • IMHO the hashing hack is reasonably fast (anyway, issymmetric is called etc.) and reliable
  • Extrapolating from their release policy I would assume that fixes will be provided in a new release which may be linked with some other features.

Once the fix is in, the hashing hack is easily removed.

@j-fu
Copy link
Collaborator Author

j-fu commented Jun 23, 2021

Seems upstream dialed back the current version to Pardiso6, so we can keep this PR on hold so far.

@KristofferC
Copy link
Member

Seems upstream dialed back the current version to Pardiso6,

What is upstream here?

@j-fu
Copy link
Collaborator Author

j-fu commented Jul 2, 2021

The guys at pardiso-project.org I am in contact with (Olaf Schenk).

I the moment, licensing and download at pardiso-project.org provide 6.0 instead of 7.2.

@KristofferC
Copy link
Member

Alright!

@j-fu
Copy link
Collaborator Author

j-fu commented May 20, 2023

Pardiso8 is out, now provided by a new startup, Panua. They now provide 12 month user and machine locked academic licenses and verify academic affiliation. The problem with the hermitian indefinite case persists, I contacted Olaf Schenk on this.

@mipals
Copy link
Member

mipals commented Jan 25, 2024

@j-fu Do you know if this code works for Pardiso8?

@j-fu
Copy link
Collaborator Author

j-fu commented Jan 25, 2024

I didn't test this yet, will try to find time.

@j-fu
Copy link
Collaborator Author

j-fu commented Jan 26, 2024

Ok got the version from panua: libpanuapardiso-20230908-linux.so .
Tests for Float64/Complex64 work for unsymmetric, hermitian posdef.
Tests fail for hermitian indef and symmetric. Didn't check deeper.

I guess for installation now we would tell users: "rename libpanuapardiso-YYYYMMDD-linux.so to libpanuapardiso.so", or is there any way to pass parameters to "deps/build.jl" ?
EDIT: Probably an environment variable...

@j-fu
Copy link
Collaborator Author

j-fu commented Jan 26, 2024

Panua changed license conditions:

Full academic licence
Unlimited number of cores.
Renewable.
Only for academic users.
Tied to a single user & node.
Duration: 365 days
Price: 300.00 CHF
Free academic licence
Limited to 2 cores.
Renewable.
Only for academic users.
Tied to a single user & node.
Duration: 30 days
Price: Free
Obtain academic trial licence

@j-fu
Copy link
Collaborator Author

j-fu commented Jan 26, 2024

Full academic cluster licence
Unlimited number of cores.
Unlimited number of nodes.
Only for academic users.
Limited to a single user & cluster.
Duration: 365 days
Price: 1200.00 CHF

@mipals
Copy link
Member

mipals commented Jan 28, 2024

Thanks a lot for checking it out Jürgen. A shame that the free license only covers two cores 😑

@mipals
Copy link
Member

mipals commented Feb 7, 2024

Hi again, now I've gotten Pardiso 8 to work on apple silicon. However, as you mentioned Jürgen the test fail for hermitian indef and symmetric. I think that there might be a problem with the try-catch for Hermitian matrices in the solve function.

@KristofferC
Copy link
Member

I am not in academia anymore so I can't really get a license and test things out. If anyone wants to take over maintenance for this package write here and I will give you commit access.

@j-fu
Copy link
Collaborator Author

j-fu commented Feb 8, 2024

I am not in academia anymore so I can't really get a license and test things out. If anyone wants to take over maintenance for this package write here and I will give you commit access.

... thinking about it ... I am in academia, and also aquainted with two of the main authors of Pardiso.

@KristofferC
Copy link
Member

I've invited you as a collaborator to the repo.

@mipals
Copy link
Member

mipals commented Feb 9, 2024

I will be in academia for at least one more year and would like to use this package meanwhile. Would be happy to help with the maintenance for now, but I am pretty inexperienced when it comes to contributing to open-source. So maybe it is enough with just Jürgen?

@KristofferC
Copy link
Member

I added you as well so you can keep an eye on each other ;)

@mipals
Copy link
Member

mipals commented Feb 9, 2024

Haha, thanks @KristofferC , sounds like a good idea 👀

@j-fu I saw in the email from Olaf that he was asking for some Julia code that reproduces the issue. Will you send him some? If not I would be happy to do it.

@j-fu
Copy link
Collaborator Author

j-fu commented Feb 9, 2024

I first wanted to rename and update this PR and try this out with Panua Pardiso (had it running already), as I ran out of pardiso8 license. If you have something ready immediately, please feel free to send it to Olaf. It feels the issue is the same since the opening of this PR (though I had no time to look more deeply into this): pardiso returns without solving and without indicating an error if it doesn't recognize the matrix type.

@mipals
Copy link
Member

mipals commented Feb 9, 2024

I have a working license already. I will send Olaf an example.

I agree that that the current issue is most likely the same issue that was at as for 7.2, i.e. that Pardiso does not return an error when it should.

@mipals
Copy link
Member

mipals commented Feb 27, 2024

Just a quick update: I've just helped Dimosthenis from Panua testing out a new binary. It seems like they have now fixed the issue on their end i.e. Panua-Pardiso now return the correct error code.

@j-fu
Copy link
Collaborator Author

j-fu commented Feb 27, 2024

So they will provide an update of their download then ?

@mipals
Copy link
Member

mipals commented Feb 27, 2024

Yes @j-fu they will provide an updated binary on their website asap. They might need to do a few more tests, but I assume that it wont take long.

README.md Outdated Show resolved Hide resolved
fixed by upstream in panua-pardiso-20240226
Make `panua_is_available`, `mkl_is_available` public interface via @compat public
- Project -> Panua
- some wording
- fix manual links for bot mkl and panua
@j-fu
Copy link
Collaborator Author

j-fu commented Feb 27, 2024

Ran PanuaPardiso tests sucessfully on 1.6, 1, nigthly, polished things a bit.

How do we release ? As 0.5.6 , 0.6 or 1.0 ?

I prefer 1.0 as this makes semver more clear.

Project.toml Outdated
MKL_jll = "2021, 2022, 2023, 2024"
Libdl= "1.6"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Formatting

Project.toml Outdated Show resolved Hide resolved
@j-fu
Copy link
Collaborator Author

j-fu commented Feb 29, 2024

Tested successfully by me on linux, and by @mipals on mac.

I tried a windows test on my vm, get an error when loading the dll:
"The specified module could not be found". Perhaps I am missing some library (the pardiso manual states that Intel compilers and vscode need to be installed).

I am lacking time to continue with the windows stuff.

How do we proceed ? Ask someone to have a test on windows ? @PetrKryslUCSD comes to my mind. AFAIK he is on windows and might be interested anyway.

@mipals
Copy link
Member

mipals commented Feb 29, 2024

I've only tested the new binary on master @j-fu . Has something changed with the loading of the binary on this branch? Should I try it out?

@j-fu
Copy link
Collaborator Author

j-fu commented Feb 29, 2024

Yes, please

@mipals
Copy link
Member

mipals commented Feb 29, 2024

Still fairly new to all of this. So I might be doing something wrong. But trying out this branch on its own does not run as it does not include the fixes for apple sillicon (as seen on master) - Meaning that I can not run the code due MKL always being loaded in.

@KristofferC
Copy link
Member

But trying out this branch on its own does not run as it does not include the fixes for apple sillicon (as seen on master)

It would have to be rebased on master to pick up those fixes. git rebase origin/master for example.

@mipals
Copy link
Member

mipals commented Feb 29, 2024

Ahh, it is there - It was just me who forgot to change to the j-fu-pardiso7 branch 😅 It seems to work on my end!

@j-fu
Copy link
Collaborator Author

j-fu commented Feb 29, 2024

The main diff now are the *_avaiable functions and the README. @mipals may be you have another look there. Guess this it will be ready to merge and to release.
I think it is non-breaking, so could be 0.5.6.

Project.toml Outdated Show resolved Hide resolved
@j-fu
Copy link
Collaborator Author

j-fu commented Mar 1, 2024

Will squash and merge in a couple of hours.

@j-fu j-fu merged commit 7bdbafc into JuliaSparse:master Mar 1, 2024
9 of 11 checks passed
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

Successfully merging this pull request may close these issues.

4 participants