search

semigroups / Semigroups

The GAP package Semigroups
https://semigroups.github.io/Semigroups/
Other
23 stars 35 forks source link

Use MarkAllButFirstSubBags for T_BIPART #1003

Closed fingolfin closed 6 months ago

fingolfin commented 6 months ago

T_BIPART consists of a general pointer not managed by GAP's GC, plus two bag references (which may be NULL). Hence we can use MarkAllButFirstSubBags instead of a custom marking function.

This prepares Semigroups for https://github.com/gap-system/gap/pull/5662, but also works in previous GAP versions.

fingolfin commented 6 months ago

(I based this on stable-5.3 but accidentally targeted main now... but the target of the PR can of course be changed as you need it, @james-d-mitchell )

james-d-mitchell commented 6 months ago

Thanks @fingolfin either branch is fine. Any idea why the ci is failing looks related to recent changes in GAP and not related to this pr at all

fingolfin commented 6 months ago

The CI failure are due to the stable-4.12 tests using the latest package distro, which has a newer GAPDoc, which makes one of the tests in stable-4.12 fail.

The ideal solution would be to test against GAP 4.12.2 (the release version) with its bundled packages, instead of against stable-4.12 plus "latest" packages. See https://github.com/gap-actions/setup-gap/issues/24.

This is something someone could look into during GAP Days next week (but realistically I think of the remaining "active" people only @james-d-mitchell, @ChrisJefferson and myself qualify... I can see if I can find a "young volunteer" to teach them, but I don't see it yet. Ah well)

A quicker workaround for the interim would be to insert a step between building GAP and running its test suite where you just remove (or update) the offending .tst file.

fingolfin commented 6 months ago

@james-d-mitchell do you think you might have time to get this into a new Semigroups release within the next couple days so we could release GAP 4.13.0 next week? Not trying to pressure you here at all, just trying to gauge what is still possible and what not :-)

james-d-mitchell commented 6 months ago

@fingolfin yes definitely, I can make a release, would like to see the CI passing first if possible. Happy to have the easy resolution of just deleting the offending files if we're using the wrong version version of GAP.

fingolfin commented 6 months ago

Basically if running the tests in stable-4.12, delete GAP's tst/testinstall/package.tst and that should settle it (or just always delete that file)