Making linear solvers independent libraries #40

sshin23 · 2021-06-18T20:49:57Z

This is a major refactoring of MadNLP.

The goal is to make the following linear solvers independent libraries:

HSL (ma27, ma57, ma77, ma86, ma97)
Mumps
Pardiso
Schur/Schwarz
LapackGPU
The independent libraries will be stored in lib.

Umfpack, PardisoMKL, LapackCPU will remain in src as a default solver, but this is subject to change.

codecov · 2021-06-23T14:40:35Z

Codecov Report

Merging #40 (b40905b) into master (4788c48) will decrease coverage by 3.04%.
The diff coverage is 88.88%.

@@            Coverage Diff             @@
##           master      #40      +/-   ##
==========================================
- Coverage   91.11%   88.07%   -3.05%     
==========================================
  Files          26       28       +2     
  Lines        2950     2959       +9     
==========================================
- Hits         2688     2606      -82     
- Misses        262      353      +91

Impacted Files	Coverage Δ
lib/MadNLPGraph/src/graphtools.jl	`100.00% <ø> (ø)`
lib/MadNLPGraph/src/schur.jl	`80.23% <ø> (ø)`
lib/MadNLPGraph/src/schwarz.jl	`76.27% <ø> (ø)`
lib/MadNLPHSL/src/ma27.jl	`76.19% <ø> (ø)`
lib/MadNLPHSL/src/ma57.jl	`75.40% <ø> (ø)`
lib/MadNLPHSL/src/ma77.jl	`88.15% <ø> (ø)`
lib/MadNLPHSL/src/ma86.jl	`84.61% <ø> (ø)`
lib/MadNLPHSL/src/ma97.jl	`83.67% <ø> (ø)`
lib/MadNLPHSL/src/mc68.jl	`100.00% <ø> (ø)`
lib/MadNLPKrylov/src/MadNLPKrylov.jl	`0.00% <ø> (ø)`
... and 31 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4788c48...b40905b. Read the comment docs.

sshin23 · 2021-06-24T04:04:05Z

@frapac Refactoring seems almost done! In this PR the following changes are made:

HSL, Mumps, LapackGPU, Pardiso, Schwarz/Schur, Krylov are now stored in separate packages. Each package is not necessarily a linear solver, but I classified them based on their practical usage (e.g., MadNLPGraphs store the Plasmo interface and graph-based solvers MadNLPSchwarz and MadNLPSchur). So now the core MadNLP has very few dependencies.
Each package has own test where the user can locally Pkg.test(package). And the CI does testing all the packages at once. The script for testing is stored in .ci
The new packages are stored in lib, as we discussed before. Laster these packages can be registered in the General julia registry.

Please feel free to check this branch and let me know what you think.

frapac

This is ❤️
The loading time of MadNLP has been reduced by a lot with this PR. I am a fan of the new structure.
I make a few comments in the PR. In short, I think we need to address:

Do we really need submodule in the linear algebra wrapper? Like defining both MadNLPGPU and MadNLPLapackGPU?
How and when to register the subpackages? This would allow to simplify the CI
I would suggest also rebasing the git commits, to avoid the long history associated with the different CI attempts

.ci/ci.jl

README.md

frapac · 2021-06-24T08:49:49Z

lib/MadNLPGPU/src/MadNLPGPU.jl

+
+include("lapackgpu.jl")
+
+export MadNLPLapackGPU


I don't think we need to define another submodule there: MadNLPLapackGPU is redundant with the parent module MadNLPGPU, which is right now an empty shell

The intention of making an empty shell is to allow further extending the MadNLPGPU package in the future. I was thinking that we may want to try using cuSOLVER's incomplete LU along with Krylov. Also, this MadNLPGPU module may also host NonlinearProgram data structures for GPU. With this approach, we don't need to have too many glue packages.

I get it, it makes total sense indeed

frapac · 2021-06-24T08:51:31Z

lib/MadNLPGPU/src/MadNLPGPU.jl

+module MadNLPGPU
+
+import CUDA: CUBLAS, CUSOLVER, CuVector, CuMatrix, toolkit_version, R_64F
+import MadNLP:


This import are quite complicated to read, in my opinion.
How about importing directly a LinearSolvers submodule?

Currently, the empty linear solver is not declared as a submodule (see here). Would there be a benefit of making it a module? We will still need to import LinearSolvers: introduce, factorize!, solver!, ...

The advantage is mostly in clarifying the code: using LinearSolvers as a submodule, we could import only the submodule and then overload directly the different functions. Then, we know directly where are the functions we are overloading (otherwise, we should check all the import at the beginning of the file to understand what we are doing exactly)

Sure, that makes sense. I'll make the changes

frapac · 2021-06-24T08:52:12Z

lib/MadNLPGPU/src/lapackgpu.jl

@@ -125,7 +125,7 @@ if toolkit_version() >= v"11.3.1"
    end
 else
    is_inertia(M::Solver) = M.opt.lapackgpu_algorithm == BUNCHKAUFMAN
-    inertia(M::Solver) = LapackCPU.inertia(M.etc[:fact_cpu],M.etc[:ipiv_cpu],M.etc[:info_cpu][])
+    inertia(M::Solver) = MadNLPLapackCPU.inertia(M.etc[:fact_cpu],M.etc[:ipiv_cpu],M.etc[:info_cpu][])


Ideally, we should overload directly the templates implemented in LinearSolvers. E.g.

LinearSolvers.inertia(M::Solver) = ...

frapac · 2021-06-24T08:55:22Z

lib/MadNLPPardiso/src/MadNLPPardiso.jl

@@ -137,8 +137,13 @@ end

 introduce(::Solver)="pardiso"

+function __init__()
+    check_deps()
+    # try


Maybe remove the dead comments?

frapac · 2021-06-24T08:56:03Z

lib/MadNLPTests/src/MadNLPTests.jl

@@ -0,0 +1,195 @@
+module MadNLPTests


This is a good idea!

frapac · 2021-06-24T08:56:19Z

src/Interfaces/interfaces.jl

@@ -2,6 +2,6 @@
 # Created by Sungho Shin (sungho.shin@wisc.edu)

 include("MOI_interface.jl")
-include("Plasmo_interface.jl")
+# include("Plasmo_interface.jl")


Remove dead comment there?

frapac · 2021-06-24T08:57:27Z

test/runtests.jl

@@ -1,14 +1,7 @@
-using Test, MadNLP, JuMP, Plasmo
+using Test, MadNLP, MadNLPTests, MINLPTests


Good idea to use MINLPTests there!

frapac · 2021-06-24T08:57:49Z

test/runtests.jl

    end

-    @testset "MadNLP test" begin 
-        include("nlp_test.jl")
+    @testset "MINNLP test" begin 


Is there a typo? MINNLP -> MINLP

sshin23 · 2021-07-02T17:24:30Z

Now all the extension packages are registered and working.

One thing though, is that MadNLPGPU is not working with CUDA 3.3. Not sure exactly why but might be because of the new DnXsytrs that was recently implemented. For now, we fall back to CUDA 3.2, and this will be addressed in a separate PR.

MadNLPHSL and MadNLPMumps created

f9e71f9

sshin23 linked an issue Jun 18, 2021 that may be closed by this pull request

Restructure dependencies in MadNLP #32

Closed

sshin23 and others added 7 commits June 18, 2021 17:39

pardiso added

fe4ceea

Making linear solvers independent libraries (#40)

a79d07a

Making linear solvers independent libraries (#40)

22003a0

Making linear solvers independent libraries #40

5b2238d

testing for each package

49186bc

test directory clean up

c18cf0e

workflow updated

07cbeda

frapac reviewed Jun 24, 2021

View reviewed changes

ci update (rebased)

9246ff5

sshin23 force-pushed the ss/refactor_linear_solvers branch 2 times, most recently from cc00a5e to f54bbb0 Compare June 27, 2021 02:29

sshin23 added 4 commits July 2, 2021 10:10

changes for registering extension packages

db4ca99

changes for registering extension packages

c794288

creating benchmark

e02e505

clean up

d035fe4

sshin23 force-pushed the ss/refactor_linear_solvers branch from a7a684e to d035fe4 Compare July 2, 2021 15:15

sshin23 added 4 commits July 2, 2021 10:16

clean up

8cfe56a

merged with master

624357f

merged with master

9cb1faf

fall back to CUDA 3.2

b40905b

sshin23 merged commit 96a2fe6 into master Jul 2, 2021

frapac deleted the ss/refactor_linear_solvers branch March 8, 2022 17:22

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Making linear solvers independent libraries #40

Making linear solvers independent libraries #40

sshin23 commented Jun 18, 2021

codecov bot commented Jun 23, 2021 •

edited

Loading

sshin23 commented Jun 24, 2021

frapac left a comment

frapac Jun 24, 2021

sshin23 Jun 25, 2021

frapac Jun 25, 2021

frapac Jun 24, 2021

sshin23 Jun 25, 2021

frapac Jun 25, 2021

sshin23 Jun 25, 2021

frapac Jun 24, 2021

frapac Jun 24, 2021

frapac Jun 24, 2021

frapac Jun 24, 2021

frapac Jun 24, 2021

frapac Jun 24, 2021

sshin23 commented Jul 2, 2021

		@@ -1,14 +1,7 @@
		using Test, MadNLP, JuMP, Plasmo
		using Test, MadNLP, MadNLPTests, MINLPTests


		include("lapackgpu.jl")

		export MadNLPLapackGPU

Making linear solvers independent libraries #40

Making linear solvers independent libraries #40

Conversation

sshin23 commented Jun 18, 2021

codecov bot commented Jun 23, 2021 • edited Loading

Codecov Report

sshin23 commented Jun 24, 2021

frapac left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sshin23 commented Jul 2, 2021

codecov bot commented Jun 23, 2021 •

edited

Loading