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

Sheaf cohomology improvements #3238

Conversation

HechtiDerLachs
Copy link
Collaborator

Two minor fixes to speed up the computation of cohomology.

CC: @wdecker

@HechtiDerLachs
Copy link
Collaborator Author

HechtiDerLachs commented Jan 24, 2024

Picking up on #2999 I now get the following timing in the example there:

julia> @time I, inc = sub(S1, f);
  0.009347 seconds (75.85 k allocations: 2.910 MiB)

More than 160 (!) times faster.

Again: nothing to do with the decision to make sub return a map (referring to what I said on #3237).

@HechtiDerLachs
Copy link
Collaborator Author

Running the tests in test/Modules/ModulesGraded.jl I get the following timings.

On current master:

52.825940 seconds (159.39 M allocations: 8.987 GiB, 6.87% gc time, 12.19% compilation time)

With the changes here:

34.451887 seconds (89.16 M allocations: 6.620 GiB, 5.44% gc time, 15.91% compilation time)

I am sure there is room for further improvements (see e.g. the compilation time!). But the minor things I did here already have a significant effect.

@HechtiDerLachs
Copy link
Collaborator Author

With the latest changes I am now down to

 30.678474 seconds (84.78 M allocations: 6.551 GiB, 5.09% gc time, 18.31% compilation time)

for test/Modules/ModulesGraded.jl.

The main issue was the following: Whenever a degree of a graded module element or an element of a graded polynomial ring was computed, the computation went on to check that really all terms have the same degree and throw an error message if this was not the case.

As far as I understand, this is alright for user facing functions and this behavior is generally desired/accepted in Oscar.

But for internal computations one must then provide a fast function which does the same thing on input which can be assumed to be sane! I made a first attempt at providing such functions, but I guess their usage still has to be spread.

That's basically the juice of it, besides avoiding the useless creation of various matrices.

P.S.: The sheaf_cohomology computation that @wdecker originally asked for (input not available on GH and therefore not referenced) is now down to

37.014906 seconds (17.35 M allocations: 704.622 MiB, 0.63% gc time)

with roughly 85% of the time spent on fres and 8% spent on the Singular-call to bgg. Thus I assume it now comes close to Singular's performance.

@HechtiDerLachs
Copy link
Collaborator Author

Fixing #2999 turns out to be more involved than first estimated.

We will eventually keep working on this PR together with @jankoboehm next week. Until then I will make a new PR containing only the minimal changes to make the sheaf cohomology computation run.

@HechtiDerLachs
Copy link
Collaborator Author

Most of this is probably addressed in other PRs which have been merged already.

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.

1 participant