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

Add 32-bit in ci #57

Merged
merged 6 commits into from
Jul 25, 2023
Merged

Add 32-bit in ci #57

merged 6 commits into from
Jul 25, 2023

Conversation

blegat
Copy link
Collaborator

@blegat blegat commented Jun 27, 2023

I don't think there is added value in testing on different OS but it's failing on 32-bit and it's making SumOfSquares fail its 32-bit build

@kalmarek
Copy link
Owner

kalmarek commented Jun 27, 2023

Why do you care about 32-bit?
I fixed one problematic line in src/Characters/GF.jl (I'm fine with that), but then sparse qr fails due to some missing CHOLMOD method with UInt32. Are you willing to dive into those?
But fixing subsequent errors requires changing UInt32 to Int32 for Ti with sparse matrices. What's the merit of using signed integers as indices in SparseMatrixCSC except using possible range?

With this change our projections will not extend beyond size ~(46340, 46340) and that's already on the edge of what I'm sometimes doing. I'm not in favour of this just for the green CI badge on 32-bit...

@blegat
Copy link
Collaborator Author

blegat commented Jun 27, 2023

No need, as long as SW compiles then it will be enough for SumOfSquares but I guess it's an issue if the tests of SW fails due to that.
Is it possible to use methods for Int32 ?

@kalmarek
Copy link
Owner

so there's a fix, but I'm still not really in favour; I will check how large are the problems we're dealing and if Int32 works for them (the size of PSD constraint is then bounded from 65535 to 46340 or so)

@kalmarek
Copy link
Owner

@piotrmizerka what are the larges psd that we solved?

@codecov
Copy link

codecov bot commented Jun 28, 2023

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (434ca6e) 87.45% compared to head (8436d76) 87.45%.

❗ Current head 8436d76 differs from pull request most recent head ce15824. Consider uploading reports for the commit ce15824 to get more accurate results

Additional details and impacted files
@@           Coverage Diff           @@
##           master      #57   +/-   ##
=======================================
  Coverage   87.45%   87.45%           
=======================================
  Files          20       20           
  Lines        1299     1299           
=======================================
  Hits         1136     1136           
  Misses        163      163           
Flag Coverage Δ
unittests 87.45% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed Coverage Δ
src/Characters/gf.jl 95.94% <100.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@kalmarek
Copy link
Owner

@blegat the current errors I'm unwilling to fix. Cyclotomics were designed to be lazy and delay normalization as long as possible. I'd rather keep it like this. This runs into troubles very quickly with decreased range of Rational{Int32}.

@blegat
Copy link
Collaborator Author

blegat commented Jun 29, 2023

I understand, I'm good with just fixing this:

ERROR: LoadError: TypeError: in typeassert, expected UInt32, got a value of type UInt64
Stacktrace:
  [1] hashindex(key::SymbolicWedderburn.Characters.FiniteFields.GF{7}, sz::Int32)
    @ Base ./dict.jl:157
  [2] ht_keyindex2_shorthash!(h::Dict{SymbolicWedderburn.Characters.FiniteFields.GF{7}, Matrix{SymbolicWedderburn.Characters.FiniteFields.GF{7}}}, key::SymbolicWedderburn.Characters.FiniteFields.GF{7})
    @ Base ./dict.jl:293
  [3] setindex!(h::Dict{SymbolicWedderburn.Characters.FiniteFields.GF{7}, Matrix{SymbolicWedderburn.Characters.FiniteFields.GF{7}}}, v0::LinearAlgebra.Transpose{SymbolicWedderburn.Characters.FiniteFields.GF{7}, Matrix{SymbolicWedderburn.Characters.FiniteFields.GF{7}}}, key::SymbolicWedderburn.Characters.FiniteFields.GF{7})
    @ Base ./dict.jl:370
  [4] left_eigen(M::Matrix{SymbolicWedderburn.Characters.FiniteFields.GF{7}})
    @ SymbolicWedderburn.Characters ~/.julia/packages/SymbolicWedderburn/inKGG/src/Characters/eigenspacedecomposition.jl:31
  [5] eigen_decomposition!(M::Matrix{SymbolicWedderburn.Characters.FiniteFields.GF{7}})
    @ SymbolicWedderburn.Characters ~/.julia/packages/SymbolicWedderburn/inKGG/src/Characters/eigenspacedecomposition.jl:53
  [6] SymbolicWedderburn.Characters.EigenSpaceDecomposition(M::Matrix{SymbolicWedderburn.Characters.FiniteFields.GF{7}})
    @ SymbolicWedderburn.Characters ~/.julia/packages/SymbolicWedderburn/inKGG/src/Characters/eigenspacedecomposition.jl:83
  [7] common_esd(Ns::Vector{SymbolicWedderburn.Characters.CMMatrix{Int32, PermutationGroups.Orbit1{PermutationGroups.Permutation{Int32, …}, Nothing}}}, F::Type{SymbolicWedderburn.Characters.FiniteFields.GF{7}})
    @ SymbolicWedderburn.Characters ~/.julia/packages/SymbolicWedderburn/inKGG/src/Characters/dixon.jl:27
  [8] SymbolicWedderburn.Characters.CharacterTable(Fp::Type{SymbolicWedderburn.Characters.FiniteFields.GF{7}}, G::PermutationGroups.PermGroup{Int32, …}, cclasses::Vector{PermutationGroups.Orbit1{PermutationGroups.Permutation{Int32, …}, Nothing}})
    @ SymbolicWedderburn.Characters ~/.julia/packages/SymbolicWedderburn/inKGG/src/Characters/character_tables.jl:62

and not modify ci

@piotrmizerka
Copy link

piotrmizerka commented Jun 29, 2023

The problems which I considered and were successfully solved - that is a certified spectral gap was obtained had about 200-400k constraints in total (that was without symmetrization using Wedderburn decomposition - with the symmetrization the numbers may be different - I don't remember now the size of psd variable matrices but they were at most several hundreds - rather closer to 100x100 than 1000x1000). There were also several problems I tried to solve but failed (in the sense that no positive gap was obtained) - they were similar in size to that problem (this is after symmetrization - I don't remember the sizes of blocks now):

problem: variables n: 448532, constraints m: 1197455
cones: z: primal zero / dual free vars: 748924
s: psd vars: 448531, ssize: 5
settings: eps_abs: 1.0e-07, eps_rel: 1.0e-07, eps_infeas: 1.0e-07
alpha: 1.50, scale: 1.00e-01, adaptive_scale: 1
max_iters: 10000, normalize: 1, rho_x: 1.00e-06
lin-sys: sparse-direct-amd-qdldl
nnz(A): 19168889, nnz(P): 0

I tried also to solve problems with 50-100 million constraints (no symmetrization here) but my impression is that the convergence seems too poor for them (I worked mostly with the SCS solver, I tried COSMO as well but I was even unable to see any iteration completed). Sample parameters here:

problem: variables n: 84766711, constraints m: 99781770
cones: z: primal zero / dual free vars: 15015060
s: psd vars: 84766710, ssize: 1
settings: eps_abs: 1.0e-06, eps_rel: 1.0e-06, eps_infeas: 1.0e-07
alpha: 1.50, scale: 1.00e-01, adaptive_scale: 1
max_iters: 1000000, normalize: 1, rho_x: 1.00e-06
acceleration_lookback: 10, acceleration_interval: 10
lin-sys: sparse-direct-amd-qdldl
nnz(A): 253733127, nnz(P): 0

@kalmarek
Copy link
Owner

@piotrmizerka yes, we're interested here in the size of the psd constraint before symmetrization; was it larger than 20k?

@piotrmizerka
Copy link

@piotrmizerka yes, we're interested here in the size of the psd constraint before symmetrization; was it larger than 20k?

If we would like to solve the problem for Delta_1 for SAutF_4 on half_radius 2, then we get 43k x 43k (S_n action) or 85k x 85k (wreath action) which clearly exceeds 20k. This I have never manage to launch.

I also tried to solve (but experiencing poor convergence so far) problems with PSD constraints forming matrices of size 14_000 x 14_000 and that was roughly 14_000^2/2 approx. 100_000_000 variables.

Another problem which was solved with prec. approx. 1e-06 (no positive spectral gap was obtained) had PSD constrain matrix of size 10_392 x 10_392.

The problems which were solved and gave positive spectral gap had PSD constraint matrices of sizes even less than 1_000 x 1000.

@kalmarek
Copy link
Owner

kalmarek commented Jul 3, 2023

@piotrmizerka did we manage to do wedderburn on the 43k×43k problem?

If not, can you try doing this again (there were multiple improvements on the "large" side of things here) and paste results here ?

@piotrmizerka
Copy link

@piotrmizerka did we manage to do wedderburn on the 43k×43k problem?

If not, can you try doing this again (there were multiple improvements on the "large" side of things here) and paste results here ?

Yes, we managed to do Wedderburn for the 43k x 43k problem. It resulted in the blocks of the following sizes: 3554, 5396, 1842, 1712, 5266. What I haven't done is running the actual solver for it - I was a bit sceptical to do that because the Wedderburn itself took about 2 weeks if I remember correctly.

@kalmarek
Copy link
Owner

kalmarek commented Jul 6, 2023

@piotrmizerka I believe that now it should go much quicker; @piotrmizerka Could you open a new issue here where you list all of those problems with their numbers & times?

kalmarek added 2 commits July 20, 2023 17:52
Indeed, it seems that we are actually hitting the typemax(Int32)
boundary in some cases and UInt32 is necessary here

This reverts commit ba9af48.
This reverts commit 6dfc1b0.
@kalmarek
Copy link
Owner

@blegat I reverted the 32bit CI; I don't intend to test the package there;
If SymbolicWedderburn doesn't load on x86 please let me know.

@kalmarek kalmarek merged commit be9c657 into master Jul 25, 2023
@kalmarek kalmarek deleted the bl/32 branch July 25, 2023 15:53
@blegat
Copy link
Collaborator Author

blegat commented Dec 5, 2023

The compiler crashes when trying to compile SW on 32-bits but it's quite unclear why:
https://github.com/jump-dev/SumOfSquares.jl/actions/runs/7101937796/job/19331181743#step:6:2570
Maybe we can wait for Julia v1.10 to see if it's fixed

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