Skip to content

Fix a bug in cohomology calculations - #169

Merged
fingolfin merged 2 commits into
gap-packages:masterfrom
stertooy:twocohom
Oct 8, 2026
Merged

fingolfin merged 2 commits into
gap-packages:masterfrom
stertooy:twocohom

Conversation

@stertooy

@stertooy stertooy commented Oct 1, 2026 •

Copy link
Copy Markdown
Collaborator

Closes #211. Built on top of #210.

The old condition was meant to check if "conjugation by the inverse of a generator" works correctly, by confirming it's the inverse operation by the (already verified) "conjugation by a generator". It checked this by testing $(x^y)^{y^{-1}} = x$ for generators $x,y$.

However, conjugating $x^y$ by $y^{-1}$ means we interpret $x^y$ as a word, and apply "conjugation by $y^{-1}$" to each of the generators appearing in this word. This means it might use the same relation multiple times, and an error in the tail could then appear an even number of times, so mod 2 it would disappear and pass the check.

Now, we instead check that $(x^{y^{-1}})^y = x$ (or rather $(x^{y^{-1}})y = yx$). Since we apply "conjugation by an inverse" to a single generator instead of a word, we can't have multiple errors cancelling out each other.

AI Disclosure: bug was discovered by GPT-6 Astra during a package audit.

@codecov

codecov Bot commented Oct 1, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 83.33333% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 67.75%. Comparing base (89138f3) to head (36f17bc).
⚠️ Report is 2 commits behind head on master.

Files with missing lines Patch % Lines
gap/cohom/twocohom.gi 83.33% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #169      +/-   ##
==========================================
+ Coverage   67.72%   67.75%   +0.02%     
==========================================
  Files          88       88              
  Lines       12914    12913       -1     
==========================================
+ Hits         8746     8749       +3     
+ Misses       4168     4164       -4     
Files with missing lines Coverage Δ
gap/cohom/twocohom.gi 74.32% <83.33%> (-0.09%) ⬇️

... and 2 files with indirect coverage changes

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

@stertooy
stertooy marked this pull request as ready for review October 7, 2026 13:51
@stertooy
stertooy requested a review from fingolfin October 7, 2026 13:52
@stertooy stertooy changed the title Deduplicate TwoCohomology code and fix a bug Fix a bug in cohomology calculations Oct 7, 2026

@fingolfin fingolfin left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I took the liberty of rebasing this on latest master, so that the diff became small.

@fingolfin
fingolfin enabled auto-merge (squash) October 8, 2026 13:52
@fingolfin
fingolfin merged commit 1f2820b into gap-packages:master Oct 8, 2026
4 checks passed
@stertooy
stertooy deleted the twocohom branch October 8, 2026 15:31
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.

Bug in calculation of 2-cohomology group

2 participants