Skip to content

Speed up arithmetic in simplicial cohomology rings - #6074

Merged
HechtiDerLachs merged 8 commits into
oscar-system:masterfrom
HechtiDerLachs:simplicial_cohomology_improvements
Aug 22, 2026
Merged

Speed up arithmetic in simplicial cohomology rings#6074
HechtiDerLachs merged 8 commits into
oscar-system:masterfrom
HechtiDerLachs:simplicial_cohomology_improvements

Conversation

@HechtiDerLachs

Copy link
Copy Markdown
Collaborator

A first take on #6066.

I cut out some uses of the OFPModule functionality over the integers, cached the result of is_zero checks, and cached the output of faces on SimplicialComplexes. This brings down the timing from the example in #6066 to

julia> @time G = matrix_space(ZZ, 22, 22)([integral(a*b) for a in g for b in g]);
 11.136035 seconds (163.40 M allocations: 4.268 GiB, 41.75% gc time, 0.75% compilation time)

for me. Ping @micjoswig , @benlorenz

@HechtiDerLachs

HechtiDerLachs commented Jun 18, 2026

Copy link
Copy Markdown
Collaborator Author

I am now at

julia> C = Oscar.SimplicialCochainComplex(ZZ, K);

julia> # automated version which seeks out a single generator for the top cohomology
       A = Oscar.DGAlgCohRing(C)
Oscar.DGAlgCohRing{ZZRingElem}(Free module of rank 16 over integer ring --> Free module of rank 120 over integer ring --> Free module of rank 560 over integer ring --> Free module of rank 720 over integer ring --> Free module of rank 288 over integer ring
, Free module of rank 1 over integer ring --> Free module of rank 0 over integer ring --> Free module of rank 22 over integer ring --> Free module of rank 0 over integer ring --> Free module of rank 1 over integer ring
, #undef, #undef, #undef, #undef, #undef)

julia> Oscar.set_volume_form!(A);

julia> g = small_generating_set(A, 2); r = 22;

julia> @time G = matrix_space(ZZ, r, r)([integral(a*b) for a in g[1:r] for b in g[1:r]])
  1.735812 seconds (28.81 M allocations: 897.702 MiB, 27.27% gc time, 4.95% compilation time)

There is a subtlety here: If you suppress the output when creating A, the simplified_cochain_complex is not computed. This seems to lead to a different small_generating_set for which computations of the intersection matrices take much longer!

However, this is by design of simplify for complexes: It computes the simplification lazy and given the information that has already been discovered. The printing triggers these starting from 0, otherwise we start at degree 2 directly. We have these phenomena often in commutative algebra. The same led to the tests timing out in until yesterday (@benlorenz probably remembers). So there's not so much we can/want to do about this, I guess. The good news is that you can get your result now within less than two seconds.

@benlorenz wanted to have another look whether some of the caching here can be done more reasonably in conjunction with polymake's internals.

@micjoswig

Copy link
Copy Markdown
Member

Improvement looks impressive. Thanks.

@HereAround HereAround added the release notes: to be added PRs introducing changes that should be (but have not yet been) mentioned in the release notes label Jul 29, 2026
@fieker

fieker commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

@HechtiDerLachs @benlorenz will discuss this for the next step

@benlorenz

Copy link
Copy Markdown
Member

I now made the simplicial complex cache the faces in a julia datatype, while we could avoid many unnecessary calls it still helps quite a bit and brings the time down to about 2 seconds now. (It took 12 seconds without the cache and using the polymake sets directly, since they still need some amount of copying to be able to do the relevant operations)

@HechtiDerLachs

Copy link
Copy Markdown
Collaborator Author

Looks good. Thanks @benlorenz !

@benlorenz

Copy link
Copy Markdown
Member

There is a weird failure in the Ring conformance tests for the DGAlgCohRing type:

https://github.com/oscar-system/Oscar.jl/actions/runs/31386085115/job/93748941519?pr=6074#step:11:4713

Basic functions: Test Failed at /home/oscarci-tester/ssd-data/ssd-runner-22/julia/packages/AbstractAlgebra/V9Znx/ext/TestExt/Rings-conformance-tests.jl:104
  Expression: equality(a + b, b + a)
   Evaluated: equality(0, 0)
  Stacktrace:
   [1] macro expansion
     @ ~/ssd-data/ssd-runner-22/_work/_tool/julia/1.13-nightly/x64/share/julia/stdlib/v1.13/Test/src/Test.jl:753 [inlined]
   [2] macro expansion
     @ ~/ssd-data/ssd-runner-22/julia/packages/AbstractAlgebra/V9Znx/ext/TestExt/Rings-conformance-tests.jl:104 [inlined]
   [3] macro expansion
     @ ~/ssd-data/ssd-runner-22/_work/_tool/julia/1.13-nightly/x64/share/julia/stdlib/v1.13/Test/src/Test.jl:1961 [inlined]
   [4] macro expansion
     @ ~/ssd-data/ssd-runner-22/julia/packages/AbstractAlgebra/V9Znx/ext/TestExt/Rings-conformance-tests.jl:73 [inlined]
   [5] macro expansion
     @ ~/ssd-data/ssd-runner-22/_work/_tool/julia/1.13-nightly/x64/share/julia/stdlib/v1.13/Test/src/Test.jl:1961 [inlined]
   [6] test_NCRing_interface(R::NCRing; reps::Any)
     @ TestExt ~/ssd-data/ssd-runner-22/julia/packages/AbstractAlgebra/V9Znx/ext/TestExt/Rings-conformance-tests.jl:26
Test Summary:                                                                                            | Pass  Fail  Total   Time
simplicial_cohomology                                                                                    | 4563     1   4564  12.0s
  NCRing interface for Oscar.DGAlgCohRing{ZZRingElem}(Free module of rank 4 over integer ring --> Free module of rank 5 over integer ring --> Free module of rank 2 over integer ring
, Free module of rank 1 over integer ring --> Free module of rank 0 over integer ring --> Free module of rank 0 over integer ring
, SubquoModule{ZZRingElem}[Submodule with 1 generator
  1: e[1] + e[2] + e[3] + e[4]
represented as subquotient with no relations, Subquotient of submodule with 3 generators
  1: -e[1] + e[3] - e[5]
  2: e[4] + e[5]
  3: e[1] + e[2]
by submodule with 4 generators
  1: -e[1] - e[2]
  2: e[1] - e[3] - e[4]
  3: e[2] + e[3] - e[5]
  4: e[4] + e[5], Subquotient of submodule with 2 generators
  1: e[1]
  2: e[2]
by submodule with 5 generators
  1: e[1]
  2: -e[1]
  3: e[1] + e[2]
  4: -e[2]
  5: e[2]], #undef, #undef, #undef, #undef, #undef) of type Oscar.DGAlgCohRing{ZZRingElem}              | 4554     1   4555   3.4s
    Functions for types and parents of rings                                                            |   23           23   1.6s
    Constructors                                                                                        |   25           25   0.0s
    Basic functions                                                                                     |  410     1    411   1.4s
    Adhoc operations with Integers                                                                      |  360          360   0.2s
    Adhoc operations with Integers{Int64}()                                                             |  360          360   0.0s
    Adhoc operations with Integers{UInt64}()                                                            |  360          360   0.0s
    Adhoc operations with Integers{UInt8}()                                                             |  360          360   0.0s
    Basic functionality for noncommutative rings only                                                   |   60           60   0.0s
    Unsafe ring operators                                                                               | 2595         2595   0.1s

This did happen twice now, but only for julia 1.13 (1.13.0-rc1.105). Once the current re-run of the failures is done I would try to restart all jobs to see if this appears anywhere else.

@codecov

codecov Bot commented Aug 11, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 91.66667% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 83.81%. Comparing base (ca196fb) to head (fc222ab).
⚠️ Report is 3 commits behind head on master.

Files with missing lines Patch % Lines
...tal/DoubleAndHyperComplexes/src/cohomology_ring.jl 90.62% 3 Missing ⚠️
...ndHyperComplexes/src/Objects/simplicial_complex.jl 87.50% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #6074      +/-   ##
==========================================
- Coverage   83.83%   83.81%   -0.03%     
==========================================
  Files         789      789              
  Lines      108512   108548      +36     
==========================================
+ Hits        90971    90977       +6     
- Misses      17541    17571      +30     
Files with missing lines Coverage Δ
src/Combinatorics/SimplicialComplexes.jl 94.19% <100.00%> (+0.23%) ⬆️
...ndHyperComplexes/src/Objects/simplicial_complex.jl 87.32% <87.50%> (-12.68%) ⬇️
...tal/DoubleAndHyperComplexes/src/cohomology_ring.jl 65.48% <90.62%> (-7.99%) ⬇️

... and 4 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@benlorenz

Copy link
Copy Markdown
Member

There is a weird failure in the Ring conformance tests for the DGAlgCohRing type:

...

This did happen twice now, but only for julia 1.13 (1.13.0-rc1.105). Once the current re-run of the failures is done I would try to restart all jobs to see if this appears anywhere else.

I can reproduce this locally, but only with 1.13-rc2 (It did not happen with 1.13-rc1 which I tried before).

Comment thread experimental/DoubleAndHyperComplexes/src/cohomology_ring.jl Outdated
@HechtiDerLachs HechtiDerLachs added release notes: use title For PRs: the title of this PR is suitable for direct use in the release notes and removed release notes: to be added PRs introducing changes that should be (but have not yet been) mentioned in the release notes labels Aug 22, 2026
@HechtiDerLachs
HechtiDerLachs merged commit 8fbd699 into oscar-system:master Aug 22, 2026
41 of 43 checks passed
@HechtiDerLachs
HechtiDerLachs deleted the simplicial_cohomology_improvements branch August 22, 2026 08:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release notes: use title For PRs: the title of this PR is suitable for direct use in the release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants