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

Registration #12

Closed
GiggleLiu opened this issue May 23, 2021 · 14 comments · Fixed by #14
Closed

Registration #12

GiggleLiu opened this issue May 23, 2021 · 14 comments · Fixed by #14

Comments

@GiggleLiu
Copy link

GiggleLiu commented May 23, 2021

Nice package. Wondering if it can be registered in the Julia registry so that I can add it as a dependancy in my package?

FYI: https://github.com/JuliaRegistries/Registrator.jl

@jalving
Copy link
Member

jalving commented May 24, 2021

Thanks for your interest! I was planning to get windows and macOS binaries working first, but who knows when that will happen.

I went ahead and made a PR to register. Once PR JuliaRegistries/General#37332 goes through, you should be able to use it as a dependency.

@GiggleLiu
Copy link
Author

GiggleLiu commented May 24, 2021

Thank you!

I was planning to get windows and macOS binaries working first

Is it an issue of the source code compiling or the binary provider?
@Gnimuc Wondering if you have any thoughts?

@jalving
Copy link
Member

jalving commented May 24, 2021

It is a matter of compiling the source code. I tried using BinaryBuilder.jl to build them locally a long time ago, but I was unsuccessful. I know it does build on macOS, and KaHyPar has been built on Windows and Appveyor at one point. A major barrier was the 1 hour time limit.

If we do get working binaries, we can just drop them under https://github.com/jalving/KaHyParBuilder which is where BinaryProvider is pulling from. I know Julia also changed to using Artifacts, but I honestly haven't looked into migrating to that system yet.

@Gnimuc
Copy link

Gnimuc commented May 24, 2021

There is already an artifact build script for KaHyPar in Ygg: https://github.com/JuliaPackaging/Yggdrasil/blob/master/K/KaHyPar/build_tarballs.jl

@jalving
Copy link
Member

jalving commented May 24, 2021

Interesting, and it looks like it built artifacts in https://github.com/JuliaBinaryWrappers/KaHyPar_jll.jl

If those work, we should update this package to use the artifact system (and probably build the latest KaHyPar with it). It looks like the creator of https://github.com/arbenson/KaHyParLite.jl set this all up to support setting different context parameters in KaHyPar. We could probably do something similar with this package as well.

@SebastianSchlag did you know about any of these packages?

@SebastianSchlag
Copy link
Member

@SebastianSchlag did you know about any of these packages?

No, I didn't know about any of those. I briefly talked with Austin Benson per Mail a while ago and he mentioned that he had adjusted the interface a bit, but I didn't know about KaHyParLite.jl.

@Roger-luo
Copy link
Contributor

Roger-luo commented May 30, 2021

for clarification and as a note for whoever wants to keep working on this, I was helping @GiggleLiu to make a new package that depends on this package work and I find the registered binary is not the official implementation (which is quite surprise!) and segment faults on my local MacOS machine and confirmed by some other users in Julia slack that it also segment faults for M1 Mac using Rosseta, so I decide to update the JLL artifact build script. I think the rest of things should be good to go after
JuliaPackaging/Yggdrasil#3111 is merged. But one may want to consider to resolve kahypar/kahypar#83 kahypar/kahypar#85 to support this package on Windows, and make the build script in Julia artifact system runnable without the patch. and right now probabbly just error on Windows, which makes our life a bit harder since we are considering using this in our quantum circuit simulator which supports Windows for free, but I guess it's acceptable for most users in short term tho.


on the other hand, I think it's also possible to support things like ARM, arch platforms with BinaryBuilder and it will be necessary since there are now things like M1, but unfortunately there are things like sse4.2 hardcoded in the CMakeFiles that I haven't got time to tweak. so might be useful for people who cares as a backlog.

@jalving
Copy link
Member

jalving commented May 30, 2021

@Roger-luo Thanks for looking into this, and for updating the build script! Indeed, we were also not aware someone had already registered their fork to host the binary source. I believe we just need to update the BinaryProvider script to pull the new builds? I can start that when your merge JuliaPackaging/Yggdrasil#3111 is complete.

As for your other suggestions, it certainly would be helpful to have a working windows build. I will have to defer to @SebastianSchlag as to the KaHyPar team's priorities with getting that to work. I figured I would try building on windows locally and upload a working binary at some point, but it would be preferable to get it working with BinaryBuilder.

For the cmake updates, feel free to file another issue. Although like you said, it might just go on the backlog.

@Roger-luo
Copy link
Contributor

Roger-luo commented May 31, 2021

I believe we just need to update the BinaryProvider script to pull the new builds?

No it's gonna be even easier, just depend on the new JLL package (after the PR is merged) and use libkahypar to replace the current lib path. I think you can delete all the BinaryProvider scripts afterwards.

@SebastianSchlag
Copy link
Member

Thanks for your comments @Roger-luo. Since we had a working Windows build at some point in time, I am confident that we should get it running again.

As for other platforms: I'd be happy to support those as well. However, since I don't have any of those available myself right now, we'd need the help of other people to try to figure out what issues currently arise on other platforms. Is sse the only thing that the M1 currently complains about?

@Roger-luo
Copy link
Contributor

Roger-luo commented Jun 1, 2021

As for other platforms: I'd be happy to support those as well. However, since I don't have any of those available myself right now, we'd need the help of other people to try to figure out what issues currently arise on other platforms. Is sse the only thing that the M1 currently complains about?

I think at least one can test this via GCC by cross compilation (tho it may still have runtime errors, but at least it would cross compile), I think you can test this on x86 machines yourself via BinaryBuilder.run_wizard() too, it is interactive, and you don't need to know much Julia to use it (persumably you can even just switch the python shared lib to this too, since it can make sure things more compatible). The current error of cross compilation to ARM is sse4.2 (it should be reproducible directly via BinaryBuilder.run_wizard), then we can prob just test it on https://github.com/JuliaPackaging/Yggdrasil/ 's CI pipeline.

Since we had a working Windows build at some point in time, I am confident that we should get it running again.

I believe the main issue for Julia side currently is that we can't build the libarry via VS2019 because Yggdrasil expect it to be cross compilable via GCC.

@Roger-luo Roger-luo mentioned this issue Jun 2, 2021
@jalving
Copy link
Member

jalving commented Jun 2, 2021

Just a heads up: once JuliaRegistries/General#37985 gets merged, v0.2.0 of the interface should be official. Thanks again @Roger-luo for setting up the jll artifacts.

@SebastianSchlag
Copy link
Member

As for other platforms: I'd be happy to support those as well. However, since I don't have any of those available myself right now, we'd need the help of other people to try to figure out what issues currently arise on other platforms. Is sse the only thing that the M1 currently complains about?

I think at least one can test this via GCC by cross compilation (tho it may still have runtime errors, but at least it would cross compile), I think you can test this on x86 machines yourself via BinaryBuilder.run_wizard() too, it is interactive, and you don't need to know much Julia to use it (persumably you can even just switch the python shared lib to this too, since it can make sure things more compatible). The current error of cross compilation to ARM is sse4.2 (it should be reproducible directly via BinaryBuilder.run_wizard), then we can prob just test it on https://github.com/JuliaPackaging/Yggdrasil/ 's CI pipeline.

Since we had a working Windows build at some point in time, I am confident that we should get it running again.

I believe the main issue for Julia side currently is that we can't build the libarry via VS2019 because Yggdrasil expect it to be cross compilable via GCC.

Thanks for the detailed explanation. I'll see what I can do to fix the sse4.2 issue.

@SebastianSchlag
Copy link
Member

I actually just removed the SSE dependency, since this it doesn't seem to be necessary anymore.

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 a pull request may close this issue.

5 participants