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

Use artifact MUMPS_jll #80

Merged
merged 6 commits into from
Aug 19, 2022
Merged

Conversation

amontoison
Copy link
Member

@amontoison amontoison commented Jan 7, 2022

close #53

@codecov
Copy link

codecov bot commented Jan 7, 2022

Codecov Report

❗ No coverage uploaded for pull request base (main@955e302). Click here to learn what that means.
Patch has no changes to coverable lines.

Additional details and impacted files
@@           Coverage Diff           @@
##             main      #80   +/-   ##
=======================================
  Coverage        ?   17.99%           
=======================================
  Files           ?       10           
  Lines           ?      889           
  Branches        ?        0           
=======================================
  Hits            ?      160           
  Misses          ?      729           
  Partials        ?        0           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@amontoison amontoison changed the title Remove old interface and use Artifacts MUMPS_jll 5.4.1 Remove old interface and use Artifacts MUMPS_jll 5.4.2 Jan 11, 2022
@dpo
Copy link
Member

dpo commented Jan 11, 2022

Let's not remove the old interface until everything works. That will also help keep this PR more focused.

@amontoison amontoison requested a review from dpo January 11, 2022 22:32
@amontoison amontoison changed the title Remove old interface and use Artifacts MUMPS_jll 5.4.2 Use artifact MUMPS_jll Jan 11, 2022
@amontoison
Copy link
Member Author

amontoison commented Jan 11, 2022

Let's not remove the old interface until everything works. That will also help keep this PR more focused.

I updated the PR to not remove the old interface except deps/build.jl.

I added a ci2.yml to test MUMPS.jl with artifact MUMPS_jll because I updated the interface such that the users can use their own installation of MUMPS if the environment variable MUMPS_PREFIX is defined.
All tests with ci.yml still use your tap dpo/mumps-jl.

@dpo
Copy link
Member

dpo commented Jan 12, 2022

I added a ci2.yml to test MUMPS.jl with artifact MUMPS_jll because I updated the interface such that the users can use their own installation of MUMPS if the environment variable MUMPS_PREFIX is defined.
All tests with ci.yml still use your tap dpo/mumps-jl.

Right, but it can no longer be built...

Also, why do we need to install all three oh OpenMPI, MPICH and MicrosoftMPI???

@amontoison
Copy link
Member Author

amontoison commented Jan 12, 2022

I added a ci2.yml to test MUMPS.jl with artifact MUMPS_jll because I updated the interface such that the users can use their own installation of MUMPS if the environment variable MUMPS_PREFIX is defined.
All tests with ci.yml still use your tap dpo/mumps-jl.

Right, but it can no longer be built...

I found a solution to keep it.

Also, why do we need to install all three oh OpenMPI, MPICH and MicrosoftMPI???

OpenMPI, MPICH and MicrosoftMPI are dependencies of MPI.jl (https://github.com/JuliaParallel/MPI.jl/blob/master/Project.toml#L10-L13).
They are also required for MPItrampoline_jll.

@dpo
Copy link
Member

dpo commented Jan 12, 2022

#81 should fix the test that fails on macOS related to accuracy.

There's still a segfault with the JLL to investigate.

Copy link
Member

@dpo dpo left a comment

Choose a reason for hiding this comment

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

It all looks great, many thanks! I would prefer to leave MUMPS.jl and mumps_types.jl unchanged in this PR and update MUMPS-C_API.jl instead.

@amontoison
Copy link
Member Author

#81 should fix the test that fails on macOS related to accuracy.

There's still a segfault with the JLL to investigate.

I don't know why there is a segfault on OSX.
I recompiled everything with Yggdrasil :(

@dpo
Copy link
Member

dpo commented Jan 13, 2022

I'll try locally.

src/MUMPS.jl Outdated Show resolved Hide resolved
src/MUMPS.jl Outdated Show resolved Hide resolved
src/MUMPS.jl Outdated Show resolved Hide resolved
src/MUMPS_C-API.jl Outdated Show resolved Hide resolved
src/MUMPS_C-API.jl Outdated Show resolved Hide resolved
@dpo
Copy link
Member

dpo commented Jan 14, 2022

ps: at least, Homebrew is also installing binaries now, which speeds up the CI (https://github.com/JuliaSmoothOptimizers/MUMPS.jl/runs/4811639965?check_suite_focus=true#step:3:59).

pps: MUMPS itself is still being compiled on Linux... not sure why.

@amontoison
Copy link
Member Author

ps: at least, Homebrew is also installing binaries now, which speeds up the CI (https://github.com/JuliaSmoothOptimizers/MUMPS.jl/runs/4811639965?check_suite_focus=true#step:3:59).

Great !

pps: MUMPS itself is still being compiled on Linux... not sure why.

MUMPS is not compiled with my ci2.yml (https://github.com/JuliaSmoothOptimizers/MUMPS.jl/runs/4811022469?check_suite_focus=true).
Maybe we should update the build name of ci.yml and ci2.yml.
Instead of CI, we could rename them CI-Homebrew and CI-Artifact.

@dpo
Copy link
Member

dpo commented Jan 22, 2022

I can reproduce the segmentation fault locally on macOS... :-(

@amontoison amontoison force-pushed the update_mumps branch 2 times, most recently from 5d7a1ac to 8e4d47d Compare January 27, 2022 18:22
@amontoison
Copy link
Member Author

amontoison commented Jan 27, 2022

@dpo I recompiled MUMPS_jll on my computer for x86_64-linux-gnu and x86_64-apple-darwin platforms without ParMETIS and deployed the new MUMPS_jll (fork amontoison/MUMPS_jll.jl).
We still have the segmentation fault on OSX :(

@dpo
Copy link
Member

dpo commented Jan 27, 2022

😞
It's hard to debug. I tried to follow the execution with dtrace and saw messages that suggested ParMETIS as a potential problem. I'll look more when I find a moment.

@amontoison
Copy link
Member Author

amontoison commented Feb 7, 2022

The version 2.2.0 of SCALAPACK was released four days ago: http://www.netlib.org/scalapack/#_scalapack_version_2_2_0
I compiled it with Yggdrasil: JuliaPackaging/Yggdrasil#4368
It could fix our issue with Mac 🤞

@amontoison
Copy link
Member Author

SCALAPACK 2.2.0 doesn't solved our issue but I found a topic about our segfault: https://developer.apple.com/forums/thread/121887
The solution is to recompile with -fno-stack-check flag.

@dpo
Copy link
Member

dpo commented Feb 8, 2022

I tried that 10 days ago, but it didn't help. Maybe I didn't recompile Scalapack with it though. I'll have to try.

@amontoison
Copy link
Member Author

@dpo MUMPS_jll works on Linux and Mac !!! 🎉

@dpo
Copy link
Member

dpo commented Aug 19, 2022

Excellent, thank you! I'm going to merge this despite the failing Homebrew actions. In the next PR, I'll remove the Homebrew actions and add M1.

@dpo dpo merged commit 92aa50c into JuliaSmoothOptimizers:main Aug 19, 2022
@amontoison
Copy link
Member Author

We can also test FreeBSD with CirrusCI :-)

@amontoison amontoison deleted the update_mumps branch August 19, 2022 19:54
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.

BinaryBuilder libraries
2 participants