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

Prepare for finite fields #12

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Conversation

j-fu
Copy link
Contributor

@j-fu j-fu commented Oct 24, 2022

triggered by https://discourse.julialang.org/t/from-toy-to-production-code-with-sparse-matrix-and-sparse-direct-factorizations/89105/13

Just a first try. Not yet ready for merge.

Main blocking things:

  • How to pass pivoting strategy ? A the moment it is gussed from the type. EDIT: We can borrow and backport the idea from Julia 1.9.
  • What does _BIGGY() really do ? In the moment I replaced it by typemax(FT) resp. one(FT) for GaloisFields. EDIT: it is used as an "uninitialized" marker. Would have to think a better way.
  • EDIT: scalable, random (?) nonsingular sparse test matrices over finite fields ?
  • EDIT: Will have to define more of the pivoting stuff missing in LinearAlgebra for 1.6
  • EDIT EDIT: Finite field packages like GaloisFields.jl need to implement lupivottype(T)=RowNonZero(), which should be no problem. However for compiling, abs, isless are needed as well. Same in the moment for Julia linear algebra. May be this can be avoided by restructuring the code.

Otherwise, for GaloisFields.jl the PR works.

Code somehow runs but is not clean

* how to handle pivoting is clear, LinearAlgebra of 1.9 provides RowNonZero() pivoting and lupivottype(T)
strategy, this needs to be used on 1.9, we need to define our own for <1.9

* As in the test example for LinearAlgebra, we need to define
  lupivottype, abs and isless for finite fields. See
  https://github.com/JuliaLang/julia/blob/1471cd9782aad18f54d8cb04316193ada15e37bd/stdlib/LinearAlgebra/test/generic.jl#L470
  Otherwise the code does not compile. May be one can avoid this by modularizing LU.

* The BIGGY problem is more serious: it occurs that BIGGY is used as an undefined marker of unititialized matrix entries.
  It is not to be expected that finite fields will have some "unitialized" value.
  Getting rid of this would need a rewrite of the input methods. This IMHO would improve the API - push! is very efficient,
  so we can rely on this, no need to estimate nnz  before.
@codecov-commenter
Copy link

codecov-commenter commented Oct 25, 2022

Codecov Report

Base: 90.31% // Head: 90.23% // Decreases project coverage by -0.07% ⚠️

Coverage data is based on head (850db55) compared to base (3e7d48f).
Patch coverage: 96.15% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #12      +/-   ##
==========================================
- Coverage   90.31%   90.23%   -0.08%     
==========================================
  Files          13       13              
  Lines        1641     1475     -166     
==========================================
- Hits         1482     1331     -151     
+ Misses        159      144      -15     
Impacted Files Coverage Δ
src/Utilities/GenericBlasLapackFragments.jl 89.21% <90.90%> (-0.02%) ⬇️
src/Problem/SpkProblem.jl 87.19% <100.00%> (-0.67%) ⬇️
src/SparseMethod/SpkLUFactor.jl 97.38% <100.00%> (-0.17%) ⬇️
src/Utilities/SpkUtilities.jl 100.00% <100.00%> (ø)
src/SparseMethod/SpkSparseBase.jl 89.26% <0.00%> (-0.49%) ⬇️
src/SparseMethod/SpkSparseSolver.jl 77.31% <0.00%> (-0.24%) ⬇️
src/SparseSpdMethod/SpkSymFct.jl 71.31% <0.00%> (-0.02%) ⬇️
... and 7 more

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.

@j-fu
Copy link
Contributor Author

j-fu commented Oct 26, 2022

See JuliaLang/julia#44571 for the discussion of the RowNonZero strategy used in this PR.

@kalmarek
Copy link

I'd be very interested to try those not only for finite fields, but also for exact fields like Cyclotomics.

Here:
https://github.com/kalmarek/SymbolicWedderburn.jl/blob/master/src/Characters/echelon_form.jl
we produced our own rref implementation which need to work for both exact entries (Rationals, GF{p} and Cyclotomics) as well as AbstractFloats.

I'd be happiest to replace all of this code by some external implementation ;)

The matrices I deal with are moderately large (~50_000×50_000), usually very low rank (25-500) and very regular in structure. They come as matrix realizations of projections in group algebras. I'm interested in obtaining any of the (L, U) factors (currently we're doing U), In the best world with orthogonal rows (or columns for L)

@j-fu
Copy link
Contributor Author

j-fu commented Nov 1, 2022

Very interesting! Never heard about Cyclotomic numbers before.

After defining abs, isless and lupivottype I was able to solve a linear system with Cyclotomic numbers with this PR.

However after your post I am not sure if this covers all your use cases. AFAIU (@PetrKryslUCSD may correct me), Sparspak is focused on the solution of nonsingular problems, so there is probably no handling of rank deficient matrices, and I also guess that adding this possibility would need quite some effort.

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.

3 participants