Fix a bug in KillingCocycle + Improve split check in ReducingCyclotom…

[angeldelriomateos]

Fix a bug in KillingCocycle to prevent a non-equivalent algebra output. This bug showed up with SG(672,622). Improve checking that a cyclic algebra (K/F,a) with a a root of unity is split by verifying is a is the norm of a root of unity simply comparing the order of a with the order of the norm of a generator of the group of roots of unity of K.

[angeldelriomateos]

Maybe this pull request is unnecessary because it is the same as PR #77

[codecov[bot]]

Codecov Report

Merging #79 into divalg will decrease coverage by 0.01%. The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           divalg      #79      +/-   ##
==========================================
- Coverage   81.59%   81.57%   -0.02%     
==========================================
  Files          11       11              
  Lines        5193     5189       -4     
==========================================
- Hits         4237     4233       -4     
  Misses        956      956              

Impacted Files	Coverage Δ
lib/div-alg.gi	87.29% <100.00%> (-0.04%)	:arrow_down:

[olexandr-konovalov]

@angeldelriomateos They are not absolutely identical though - the change in line 1961 is new here, and diffs coverage is higher. Perhaps #78 is incorrect. But either your commit message should be polished, or I should change line 1961 in #78. Let's look at this tomorrow.

[olexandr-konovalov]

@angeldelriomateos also, unless these two changes are interconnected, they should rather be done in two different commits. This could help in the future to traverse the history and find where bugs were introduced. Although we hope that our code is perfect and has no bugs, we know that they do happen! :)

[angeldelriomateos]

@angeldelriomateos They are not absolutely identical though - the change in line 1961 is new here, and diffs coverage is higher. Perhaps #78 is incorrect. But either your commit message should be polished, or I should change line 1961 in #78. Let's look at this tomorrow.

Line 1961 should be as in PR #79, i.e. for j in [1..k] do

[olexandr-konovalov]

@angeldelriomateos

Fix a bug in KillingCocycle to prevent a non-equivalent algebra output. This bug showed up with SG(672,622).

Do we have an example which fails in the current divalg and passes with your fixes?

[angeldelriomateos]

@angeldelriomateos

Fix a bug in KillingCocycle to prevent a non-equivalent algebra output. This bug showed up with SG(672,622).

Do we have an example which fails in the current divalg and passes with your fixes?

Yes, calculating LocalIndicesOfCyclotomicAlgebra of the last component of WedderburnDecompositionInfo(GroupRing(Rationals,SmallGroup(672,622))) produces an error without this fix and passes with this fix

[olexandr-konovalov]

Ok, that I can reproduce. I've updated the test in #78 to include that.

[olexandr-konovalov]

@angeldelriomateos #78 now has all changes from here and edited commit message. If you think that #78 is ready, I will rebase it on top of just merged #81, run tests again, merge #78 if tests pass, and just close this PR.

[olexandr-konovalov]

Closed in favour of #78.