Closed olexandr-konovalov closed 4 years ago
Merging #81 into divalg will increase coverage by
0.71%
. The diff coverage isn/a
.
@@ Coverage Diff @@
## divalg #81 +/- ##
==========================================
+ Coverage 81.59% 82.30% +0.71%
==========================================
Files 11 11
Lines 5193 5193
==========================================
+ Hits 4237 4274 +37
+ Misses 956 919 -37
Impacted Files | Coverage Δ | |
---|---|---|
lib/div-alg.gi | 89.53% <0.00%> (+2.21%) |
:arrow_up: |
So, new examples suggested by @drallenherman do not cover more lines, but they take ~20s in total, so if these cases are interesting and may be used to check performance or memory regressions, or may check some further modifications of code, treating special cases, we still can add them. Any thoughts?
So, new examples suggested by @drallenherman do not cover more lines, but they take ~20s in total, so if these cases are interesting and may be used to check performance or memory regressions, or may check some further modifications of code, treating special cases, we still can add them. Any thoughts?
I think we should find examples that increase coverage. I'll look at the specific lines not being covered and try to come up with better ones.
@drallenherman ok, then please post examples here, and I will update and force-push this PR to see if they have any effect.
So, new examples suggested by @drallenherman do not cover more lines, but they take ~20s in total, so if these cases are interesting and may be used to check performance or memory regressions, or may check some further modifications of code, treating special cases, we still can add them. Any thoughts?
I think we should find examples that increase coverage. I'll look at the specific lines not being covered and try to come up with better ones.
Quite a few lines of div-alg that lack code coverage are in DecomposeCyclotomicAlgebra. If we apply that to the last component of QG for SG(240,96) it will improve coverage. Especially after branch #80 gets merged.
Nice @drallenherman !
# Some special cases in DecomposeCyclotomicAlgebra (PR #81)
gap> QG:=GroupRing(Rationals,SmallGroup(240,96));;
gap> W:=WedderburnDecompositionInfo(QG);;
gap> Length(W);
18
gap> A:=W[Length(W)];
[ 1, Rationals, 30, [ [ 2, 11, 0 ], [ 4, 7, 0 ] ], [ [ 15 ] ] ]
gap> DecomposeCyclotomicAlgebra(A);
[ [ Rationals, CF(3), [ 1 ] ], [ Rationals, CF(5), [ 0 ] ] ]
hits 37 more lines! Going to merge it.
So, new examples suggested by @drallenherman do not cover more lines, but they take ~20s in total, so if these cases are interesting and may be used to check performance or memory regressions, or may check some further modifications of code, treating special cases, we still can add them. Any thoughts?
I think we should find examples that increase coverage. I'll look at the specific lines not being covered and try to come up with better ones.
Quite a few lines of div-alg that lack code coverage are in DecomposeCyclotomicAlgebra. If we apply that to the last component of QG for SG(240,96) it will improve coverage. Especially after branch #80 gets merged.
Nice @drallenherman !
# Some special cases in DecomposeCyclotomicAlgebra (PR #81) gap> QG:=GroupRing(Rationals,SmallGroup(240,96));; gap> W:=WedderburnDecompositionInfo(QG);; gap> Length(W); 18 gap> A:=W[Length(W)]; [ 1, Rationals, 30, [ [ 2, 11, 0 ], [ 4, 7, 0 ] ], [ [ 15 ] ] ] gap> DecomposeCyclotomicAlgebra(A); [ [ Rationals, CF(3), [ 1 ] ], [ Rationals, CF(5), [ 0 ] ] ]
hits 37 more lines! Going to merge it.
All right. We need to merge #80 next to bring in the improved version of DecomposeCA. When you do that you will need to change the [0] at the end of this one to a [9]. It's likely coverage will go down slightly it's hard to cover all of the cases in the new DecomposeCA.
We need to merge #80 next to bring in the improved version of DecomposeCA. When you do that you will need to change the [0] at the end of this one to a [9].
No - you will need to rebase your #80 (what you need to do anyway) and update the test there.
Attempts to find examples with increased coverage. Trying suggestions by @drallenherman from the discussion in #76: