Add a Valgrind GitHub Actions workflow

[wilfwilson]

This is basically a copy of the file in Digraphs: thank you whoever made that!

To see that the workflow functions as expected, see here: https://github.com/wilfwilson/Semigroups/actions/runs/17740854488/job/50414239468

Note that the job fails, because Valgrind does indeed find some issues...

[james-d-mitchell]

I'm not seeing the valgrind job actually running, is that just me?

[james-d-mitchell]

I'm not seeing the valgrind job actually running, is that just me?

Oh right it's only triggered by changes in the kernel module, any chance you can make a white space change so we can see it failing?

[wilfwilson]

@james-d-mitchell It's running! Fun times.

[wilfwilson]

Looks like it failed after running testinstall with acting methods off... with 2834 errors 😱

[james-d-mitchell]

Looks like it failed after running testinstall with acting methods off... with 2834 errors 😱

Zoiks that is not a small number

[wilfwilson]

Hopefully it's the same one error happening 2834 times 🤞

[james-d-mitchell]

I'll have a look on Wednesday

[wilfwilson]

@james-d-mitchell I suggest that you give me sufficient permissions on this repository such that I can fiddle with the respository settings vis-a-vis GitHub Actions.

I propose that we merge this PR, but have the settings for the Valgrind CI job to not fail the PR status checks. i.e. it runs when necessary, and gives a pass/fail status on the relevant job, but it doesn't stop the PR from being merged, and on the lists of pull requests, it doesn't stop a PR from still having a green tick next to it (if everything is fine).

Once we've addressed all of the relevant issues (ahem), we can become strict about making sure that no PR introduces new Valgrind issues.

[Joseph-Edwards]

I've just tried to replicate this valgrind job locally, and my PC is consistently running out of memory whilst running the standard tests with valgrind enabled.

I'm going to do some digging to see if I can find the tests that cause this, and will report back.

[james-d-mitchell]

Thanks @Joseph-Edwards, we could also just run the quick tests, I don't think it's reasonable to expect all the tests to run in a reasonable amount of time/memory with valgrind enabled.

[Joseph-Edwards]

I ran the valgrind job again, this time omitting the tests in tst/standard/fp/tietze.tst. Valgrind managed to complete without running out of memory, and exited with this:

==181341== LEAK SUMMARY:
==181341==    definitely lost: 0 bytes in 0 blocks
==181341==    indirectly lost: 0 bytes in 0 blocks
==181341==      possibly lost: 808 bytes in 8 blocks
==181341==    still reachable: 67,211,879 bytes in 163,635 blocks
==181341==                       of which reachable via heuristic:
==181341==                         stdstring          : 232 bytes in 4 blocks
==181341==         suppressed: 32 bytes in 1 blocks
==181341==
==181341== ERROR SUMMARY: 4 errors from 4 contexts (suppressed: 0 from 0)
--181341--
--181341-- used_suppression:      1 dtv-addr-init /usr/libexec/valgrind/default.supp:1581 suppressed: 32 bytes in 1 blocks
==181341==
==181341== ERROR SUMMARY: 4 errors from 4 contexts (suppressed: 0 from 0)

The full output can be found here.

I've tried to wrap my head around the 4 errors that we are receiving. I think they're related to the 808 bytes that valgrind claims are "possibly lost". There's a good chance that these are false positives, so we might be best to ignore/suppress them.

From here, we have a few questions we may wish to answer:

1. Why do the tests in tst/standard/fp/tietze.tst consume so much memory when run in valgrind?
2. Are those 808 bytes actually lost?
3. How do we proceed with integrating valgrind into the CI?

Questions 1. and 2. may not be important enough to warrant exploring at the moment.

For question 3., I see a few reasonable options. The first is to not run valgrind on the standard tests at all. Another potential solution is to run valgrind on most of the standard tests apart from the ones in the offending Tietze file, only checking for "definitely lost" and "indirectly lost" leaks.

I think I prefer the second of these two cases, simply because it tests more things. I'm happy to hear other suggestions for how to proceed.

[wilfwilson]

Your preferred option sounds good Joe

[Joseph-Edwards]

After chatting to James, we are in agreement that valgrind is taking too long to be run on every pull request. Instead, we should only run the standard tests when changes to the kernel have been made. I'll try and make that change at some point tomorrow.

[Joseph-Edwards]

For what it's worth, this is the time spent running each test file:

semigroups/semirms.tst              32 mins 18 secs
attributes/isorms.tst               19 mins 49 secs
greens/acting.tst                   14 mins 35 secs
attributes/attr.tst                 10 mins 20 secs
greens/froidure-pin.tst              9 mins 36 secs
attributes/translat.tst              8 mins  7 secs
attributes/properties.tst            5 mins 17 secs
attributes/factor.tst                5 mins  1 sec
semigroups/semigrp.tst               4 mins 58 secs
attributes/homomorph.tst             4 mins 51 secs
semigroups/semicons.tst              4 mins 30 secs
semigroups/semiffmat.tst             4 mins 28 secs
semigroups/semidp.tst                4 mins 22 secs
semigroups/semifp.tst                4 mins 22 secs
attributes/isomorph.tst              4 mins  0 secs
semigroups/semitrans.tst             3 mins 44 secs
ideals/ideals.tst                    3 mins 43 secs
semigroups/semipperm.tst             2 mins 54 secs
greens/generic.tst                   2 mins 52 secs
semigroups/semiex.tst                2 mins 18 secs
congruences/conglatt.tst             2 mins  5 secs
semigroups/semibipart.tst            2 mins  3 secs
greens/acting-inverse.tst            1 min  38 secs
semigroups/semiboolmat.tst           1 min  38 secs
fp/freeinverse.tst                   1 min  37 secs
attributes/maximal.tst               1 min  37 secs
semigroups/semieunit.tst             1 min  37 secs
ideals/acting.tst                    1 min  23 secs
semigroups/semipbr.tst               1 min  19 secs
main/froidure-pin.tst                1 min  18 secs
attributes/dual.tst                  1 min  12 secs
fp/freeband.tst                      1 min  11 secs
congruences/conginv.tst              1 min  10 secs
attributes/inverse.tst               1 min   8 secs
main/acting.tst                      1 min   5 secs
semigroups/semimaxplus.tst           1 min   2 secs
congruences/cong.tst                 1 min   0 secs
tools/display.tst                    0 mins 59 secs
congruences/congpairs.tst            0 mins 59 secs
congruences/congrees.tst             0 mins 58 secs
semigroups/semiquo.tst               0 mins 56 secs
main/semiact.tst                     0 mins 54 secs
elements/trans.tst                   0 mins 51 secs
libsemigroups/froidure-pin.\         0 mins 47 secs
libsemigroups/sims1.tst              0 mins 47 secs
congruences/congrms.tst              0 mins 39 secs
elements/bipart.tst                  0 mins 38 secs
attributes/cartan.tst                0 mins 37 secs
main/setup.tst                       0 mins 36 secs
attributes/acting.tst                0 mins 36 secs
greens/acting-regular.tst            0 mins 35 secs
congruences/congsemigraph.t\         0 mins 34 secs
congruences/congsimple.tst           0 mins 34 secs
tools/io.tst                         0 mins 34 secs
libsemigroups/cong.tst               0 mins 33 secs
semigroups/grpperm.tst               0 mins 32 secs
semigroups/semigraph.tst             0 mins 32 secs
elements/ffmat.tst                   0 mins 32 secs
congruences/congwordgraph.t\         0 mins 31 secs
elements/semiringmat.tst             0 mins 29 secs
elements/maxplusmat.tst              0 mins 29 secs
congruences/conguniv.tst             0 mins 28 secs
elements/pbr.tst                     0 mins 27 secs
ideals/froidure-pin.tst              0 mins 27 secs
elements/boolmat.tst                 0 mins 26 secs
elements/blocks.tst                  0 mins 26 secs
attributes/semifp.tst                0 mins 26 secs
elements/elements.tst                0 mins 25 secs
options.tst                          0 mins 25 secs
obsolete.tst                         0 mins 23 secs
elements/pperm.tst                   0 mins 23 secs
fp/word.tst                          0 mins 22 secs

[james-d-mitchell]

I was going to say, maybe it's only worthwhile running the tests in tst/standard/libsemigroups in this action, but those are absent from your list. @Joseph-Edwards