gap-packages / recog

The GAP package recog to collect methods for constructive recognition
https://gap-packages.github.io/recog/
GNU General Public License v3.0
6 stars 14 forks source link

Replace SporadicByOrders by NameSporadic #197

Closed ssiccha closed 3 years ago

ssiccha commented 4 years ago

Fixes #196.

ssiccha commented 4 years ago

Unfortunately, running the old version of the Sporadic.tst, that is the version without the tests that were previously broken, but with excluded extensions like Fi22.2, now takes 13 seconds instead of 2 seconds. Though now the results should be correct more often. :-)

I'm running 50 iterations of the Sporadic.tst on my machine now. Let's see how many runs fail.

codecov[bot] commented 4 years ago

Codecov Report

Merging #197 (c543566) into master (5ec87e6) will decrease coverage by 0.91%. The diff coverage is 71.45%.

:exclamation: Current head c543566 differs from pull request most recent head e7c5154. Consider uploading reports for the commit e7c5154 to get more accurate results

@@            Coverage Diff             @@
##           master     #197      +/-   ##
==========================================
- Coverage   78.68%   77.77%   -0.92%     
==========================================
  Files          37       37              
  Lines       17183    17490     +307     
==========================================
+ Hits        13521    13602      +81     
- Misses       3662     3888     +226     
Impacted Files Coverage Δ
gap/base/methsel.gi 94.80% <ø> (ø)
gap/matrix/slconstr.gi 5.88% <0.00%> (-0.03%) :arrow_down:
gap/tools.gi 28.57% <0.00%> (-4.16%) :arrow_down:
gap/base/recognition.gi 71.12% <32.14%> (-1.61%) :arrow_down:
gap/projective/findnormal.gi 78.10% <53.33%> (-0.86%) :arrow_down:
gap/projective/classicalnatural.gi 92.49% <61.11%> (-0.72%) :arrow_down:
gap/perm/largebase.gi 82.37% <62.50%> (-1.03%) :arrow_down:
gap/matrix.gi 91.14% <66.66%> (-0.16%) :arrow_down:
gap/matrix/ppd.gi 93.27% <66.66%> (-0.74%) :arrow_down:
gap/projective/AnSnOnFDPM.gi 78.00% <66.66%> (-3.37%) :arrow_down:
... and 15 more
ssiccha commented 4 years ago

If we want to merge this I can remove the SporadicsByOrders function and all of its, not otherwise used, subfunctions.

ssiccha commented 4 years ago

Out of the 50 iterations, all passed on my machine.

ssiccha commented 4 years ago

I still need to actually use NameSporadic in the recognition of leaves.

ssiccha commented 4 years ago

I forgot to update the local statement.

ssiccha commented 3 years ago

Rebased and finished. Let's see whether the tests pass.

ssiccha commented 3 years ago

Tests now pass on my machine.

ssiccha commented 3 years ago

Rebased onto master and enabled sporadic tests again. @fingolfin I didn't squash the changes yet so that it is easier for you to see my latest changes. Except for that I this PR is ready from my POV.

ssiccha commented 3 years ago

@fingolfin Could you have a look again? From my POV this is ready to be squashed and merged.