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

Speed up SecondPageOfCohomologicalSpectralSequence.g #542

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

zickgraf
Copy link
Member

by compiling AdditiveClosure( QuotientCategory( LinearClosure ) )

@mohamed-barakat A few things:

  1. Currently, I load the compiled code manually in the example file because there are so many different versions of QuotientCategory that I am not sure for which versions the compiled code is actually valid. For loading it automatically, you can set the compiler hint precompiled_towers (see e.g. https://github.com/homalg-project/Algebroids/blob/277ef58afcc40ecf01d9dda51331b3f89b7f19be/gap/Algebroids.gi#L1193) in the category constructor for which the compiled code is valid.
  2. Please review the type signatures. Due to missing documentation this was a bit of guesswork.
  3. There still is a warning about a missing type declaration, which does however not affect the compiled code (the tree cannot be typed anyway due to using Position, see the next point).
  4. The runtime is improved by a factor > 10, but still is about 45 seconds on my laptop. I think more could be gained by improving/refactoring https://github.com/homalg-project/Algebroids/blob/277ef58afcc40ecf01d9dda51331b3f89b7f19be/gap/QuotientsOfLinearClosuresOfPathCategories.gi#L117. The filename QuotientsOfLinearClosuresOfPathCategories sounds very specific, I guess one approach could be to first make this case more generic.

@zickgraf zickgraf marked this pull request as draft June 26, 2024 16:25
Copy link

codecov bot commented Jun 26, 2024

Codecov Report

Attention: Patch coverage is 84.61538% with 2 lines in your changes missing coverage. Please review.

Project coverage is 76.09%. Comparing base (b313f3b) to head (4bb1088).

Files Patch % Lines
...LinearClosuresOfPathCategoriesAndTheirQuotients.gd 60.00% 2 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##           master     #542   +/-   ##
=======================================
  Coverage   76.09%   76.09%           
=======================================
  Files         389      389           
  Lines       58034    58044   +10     
=======================================
+ Hits        44161    44169    +8     
- Misses      13873    13875    +2     
Flag Coverage Δ
Algebroids 76.05% <80.00%> (-0.01%) ⬇️
CatReps 83.54% <ø> (ø)
CategoriesWithAmbientObjects 89.28% <ø> (ø)
ExteriorPowersCategories 43.70% <ø> (ø)
FiniteCocompletions 92.69% <ø> (ø)
FpCategories 88.43% <ø> (ø)
FunctorCategories 65.40% <ø> (ø)
GradedCategories 86.57% <ø> (ø)
InternalModules 81.66% <ø> (ø)
IntrinsicCategories 82.55% <ø> (ø)
IntrinsicGradedModules 50.83% <ø> (ø)
IntrinsicModules 82.15% <ø> (ø)
LazyCategories 68.38% <ø> (ø)
Locales 86.00% <ø> (ø)
PreSheaves 91.92% <ø> (ø)
QuotientCategories 92.15% <100.00%> (+0.06%) ⬆️
SubcategoriesForCAP 81.25% <ø> (ø)
ToolsForCategoricalTowers 84.93% <ø> (ø)
Toposes 90.86% <ø> (ø)
ZariskiFrames 74.31% <ø> (ø)

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

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

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.

None yet

1 participant