-
Notifications
You must be signed in to change notification settings - Fork 27
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
Conversation
Is there no error code at all or something else that can be inspect for this? |
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... |
The PR hashes X before and after, and figures this out (so runtests works with on my system), but this is not optimal. |
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.
Once the fix is in, the hashing hack is easily removed. |
Seems upstream dialed back the current version to Pardiso6, so we can keep this PR on hold so far. |
What is upstream here? |
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. |
Alright! |
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. |
@j-fu Do you know if this code works for Pardiso8? |
I didn't test this yet, will try to find time. |
Ok got the version from panua: 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" ? |
Panua changed license conditions:
|
|
Thanks a lot for checking it out Jürgen. A shame that the free license only covers two cores 😑 |
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. |
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. |
I've invited you as a collaborator to the repo. |
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? |
I added you as well so you can keep an eye on each other ;) |
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. |
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. |
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. |
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. |
So they will provide an update of their download then ? |
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. |
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
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" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Formatting
* Panua everywhere * fixed more links to Panua, MKL
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: 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. |
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? |
Yes, please |
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. |
It would have to be rebased on master to pick up those fixes. |
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! |
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. |
Co-authored-by: Kristoffer Carlsson <[email protected]>
Will squash and merge in a couple of hours. |
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.