From 452f5f5381a9ca3fa6a378d15be0f32cc5f1fe53 Mon Sep 17 00:00:00 2001 From: Max Horn Date: Sun, 16 Aug 2026 19:07:47 +0200 Subject: [PATCH 1/3] Make the tests independent of polymake's convex hull backend polymake enumerates facets in an order that depends on which convex hull backend it picks -- ppl, cdd, lrs or the built-in beneath_beyond -- and FACETS rows are only determined up to a positive scalar. Four expectations in polymaking.tst encoded one particular backend's answer, so the suite failed for anyone whose polymake was built without ppl. Canonicalise before comparing: sort faces by dimension and then lexicographically, scale each FACETS row to a primitive integer vector keeping its sign, and sort the facet list. ADJACENCY, whose node numbering follows the same facet order, is instead checked structurally: it is the covering relation of FACES, which the same call returns in the same node order. Also stop the suite from rewriting files it does not own. It pointed the data directory at the package's own tst directory and read pplane.poly from there, which polymake upgrades in place, so running the tests dirtied the working tree and made a second run test a different input. Copy to a temporary directory first, and let CI catch a regression with git diff --exit-code. Set POLYMAKING_CHULL to test a specific backend, e.g. POLYMAKING_CHULL=cdd gap tst/testall.g. CI now covers cdd and beneath_beyond in addition to the default. While here, repair visual.tst, which had rotted while excluded from the suite. Fixes #18. Co-Authored-By: Claude Opus 5 --- .github/workflows/CI.yml | 15 ++++++- CHANGES.md | 4 ++ tst/polymaking.tst | 94 +++++++++++++++++++++++++++------------- tst/testall.g | 12 +++++ tst/userprefs.tst | 14 +++--- tst/visual.tst | 14 +++--- 6 files changed, 111 insertions(+), 42 deletions(-) diff --git a/.github/workflows/CI.yml b/.github/workflows/CI.yml index 8143242..c906412 100644 --- a/.github/workflows/CI.yml +++ b/.github/workflows/CI.yml @@ -19,7 +19,7 @@ concurrency: jobs: # The CI test job test: - name: ${{ matrix.gap-version }} + name: ${{ matrix.gap-version }} / ${{ matrix.chull }} runs-on: ubuntu-latest strategy: fail-fast: false @@ -28,6 +28,15 @@ jobs: - 'devel' # current GAP development version from git - 'latest' # latest GAP release - 'minimal' # oldest GAP release supported by this package + # polymake enumerates facets in a backend dependent order, so run the + # tests against more than one; beneath_beyond is built into polymake and + # always available, cdd is what Debian and Gentoo builds tend to use. + chull: [ 'default' ] + include: + - gap-version: 'latest' + chull: 'cdd' + - gap-version: 'latest' + chull: 'beneath_beyond' steps: - uses: actions/checkout@v6 @@ -36,6 +45,10 @@ jobs: gap-version: ${{ matrix.gap-version }} - uses: gap-actions/build-pkg@v3 - uses: gap-actions/run-pkg-tests@v4 + env: + POLYMAKING_CHULL: ${{ matrix.chull != 'default' && matrix.chull || '' }} + - name: "Fail if the tests modified checked-in files" + run: git diff --exit-code - uses: gap-actions/run-pkg-tests@v4 with: mode: onlyneeded diff --git a/CHANGES.md b/CHANGES.md index e64ccd0..2e7b335 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -18,6 +18,10 @@ - `POLYMAKE_LAST_FAIL_REASON` is now actually set when polymake fails; it used to be assigned after the `Error` call, and so only when the user resumed from the break loop +- the test suite no longer depends on which convex hull backend polymake picks, + and no longer rewrites `tst/pplane.poly` in place while running (issue #18). + Set `POLYMAKING_CHULL` to test a specific backend, e.g. + `POLYMAKING_CHULL=cdd gap tst/testall.g`. - the globals `POLYMAKE_COMMAND` and `POLYMAKE_DATA_DIR` are no longer set by the package; if you set them yourself they are still honoured diff --git a/tst/polymaking.tst b/tst/polymaking.tst index 9fc13a2..c98702c 100644 --- a/tst/polymaking.tst +++ b/tst/polymaking.tst @@ -1,5 +1,33 @@ gap> START_TEST("polymaking tst file"); +## The order in which polymake enumerates facets depends on the convex hull +## backend it picks (ppl, cdd, lrs, beneath_beyond, ...), and FACETS rows are +## only determined up to a positive scalar. Canonicalise before comparing. +## +gap> NormalizeRow := function(v) +> local d, g; +> d := Lcm(List(v, DenominatorRat)); +> v := v * d; +> g := Gcd(v); +> if g <> 0 then v := v / g; fi; +> return v; +> end;; +gap> CanonicalFaceList := function(L) +> local c; +> c := ShallowCopy(L); +> Sort(c, function(a, b) return [Size(a), a] < [Size(b), b]; end); +> return c; +> end;; +gap> CoversOfFaceList := function(faces) +> local n, lt; +> n := Length(faces); +> lt := function(i, j) +> return faces[i] <> faces[j] and IsSubset(faces[j], faces[i]); +> end; +> return List([1..n], i -> Filtered([1..n], +> j -> lt(i, j) and not ForAny([1..n], k -> lt(i, k) and lt(k, j)))); +> end;; + ## generate an empty file> ## gap> poly:=CreatePolymakeObject(); @@ -53,39 +81,39 @@ gap> Polymake(poly,"F_VECTOR"); [ 9, 21, 14 ] gap> Polymake(poly,"F2_VECTOR"); [ [ 9, 42, 42 ], [ 42, 21, 42 ], [ 42, 42, 14 ] ] -gap> Polymake(poly,"FACES"); +gap> faces := Polymake(poly,"FACES");; +gap> CanonicalFaceList(faces); [ [ ], [ 1 ], [ 2 ], [ 3 ], [ 4 ], [ 5 ], [ 6 ], [ 7 ], [ 8 ], [ 9 ], [ 1, 2 ], [ 1, 4 ], [ 1, 6 ], [ 1, 8 ], [ 2, 4 ], [ 2, 6 ], [ 2, 9 ], [ 3, 4 ], [ 3, 5 ], [ 3, 7 ], [ 3, 9 ], [ 4, 5 ], [ 4, 7 ], [ 4, 8 ], [ 4, 9 ], [ 5, 9 ], [ 6, 7 ], [ 6, 8 ], [ 6, 9 ], [ 7, 8 ], [ 7, 9 ], - [ 1, 6, 8 ], [ 2, 4, 9 ], [ 3, 4, 7 ], [ 6, 7, 8 ], [ 4, 7, 8 ], - [ 3, 7, 9 ], [ 6, 7, 9 ], [ 1, 4, 8 ], [ 1, 2, 4 ], [ 2, 6, 9 ], - [ 1, 2, 6 ], [ 3, 5, 9 ], [ 4, 5, 9 ], [ 3, 4, 5 ], + [ 1, 2, 4 ], [ 1, 2, 6 ], [ 1, 4, 8 ], [ 1, 6, 8 ], [ 2, 4, 9 ], + [ 2, 6, 9 ], [ 3, 4, 5 ], [ 3, 4, 7 ], [ 3, 5, 9 ], [ 3, 7, 9 ], + [ 4, 5, 9 ], [ 4, 7, 8 ], [ 6, 7, 8 ], [ 6, 7, 9 ], [ 1, 2, 3, 4, 5, 6, 7, 8, 9 ] ] -gap> Polymake(poly,"ADJACENCY"); -[ [ 2, 3, 4, 5, 6, 7, 8, 9, 10 ], [ 11, 12, 13, 14 ], [ 11, 15, 16, 17 ], - [ 18, 19, 20, 21 ], [ 12, 15, 18, 22, 23, 24, 25 ], [ 19, 22, 26 ], - [ 13, 16, 27, 28, 29 ], [ 20, 23, 27, 30, 31 ], [ 14, 24, 28, 30 ], - [ 17, 21, 25, 26, 29, 31 ], [ 40, 42 ], [ 39, 40 ], [ 32, 42 ], [ 32, 39 ], - [ 33, 40 ], [ 41, 42 ], [ 33, 41 ], [ 34, 45 ], [ 43, 45 ], [ 34, 37 ], - [ 37, 43 ], [ 44, 45 ], [ 34, 36 ], [ 36, 39 ], [ 33, 44 ], [ 43, 44 ], - [ 35, 38 ], [ 32, 35 ], [ 38, 41 ], [ 35, 36 ], [ 37, 38 ], [ 46 ], [ 46 ], - [ 46 ], [ 46 ], [ 46 ], [ 46 ], [ 46 ], [ 46 ], [ 46 ], [ 46 ], [ 46 ], - [ 46 ], [ 46 ], [ 46 ], [ ] ] +gap> adj := Polymake(poly,"ADJACENCY");; +gap> Length(adj) = Length(faces); +true +gap> List(adj, Set) = CoversOfFaceList(faces); +true +gap> Collected(List(adj, Length)); +[ [ 0, 1 ], [ 1, 14 ], [ 2, 21 ], [ 3, 1 ], [ 4, 4 ], [ 5, 2 ], [ 6, 1 ], + [ 7, 1 ], [ 9, 1 ] ] gap> Polymake(poly,"FACET_DEGREES"); [ 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3 ] -gap> Polymake(poly,"FACETS"); -[ [ 87, -452, 1800, 44 ], [ -42443748, 474787219, 1685061792, 46231655 ], - [ 1403725, 643437067, -190478253, -47332395 ], - [ 27853, -108309, -630630, 351252 ], - [ 13885273, -1477537, -280945070, -48335100 ], - [ -145029, 1373099, 1147776, 7226208 ], - [ -235790, -456383, 2076480, 14126013 ], [ 23349, -103132, 738675, -163130 ] - , [ -1580061339, 1195816838, 93208905900, 843077785 ], - [ -2006700, 2225735, 88056864, 9999187 ], [ -51826, 38850, 2915550, 71269 ], - [ -574215, 16096535, 4705056, 17713168 ], +gap> Set(Polymake(poly,"FACETS"), NormalizeRow); +[ [ -2303641863, 164204234285, 12897123192, -3359875640 ], + [ -1580061339, 1195816838, 93208905900, 843077785 ], [ -519350247, 28577972485, 5781969216, -256601080 ], - [ -2303641863, 164204234285, 12897123192, -3359875640 ] ] + [ -42443748, 474787219, 1685061792, 46231655 ], + [ -2006700, 2225735, 88056864, 9999187 ], + [ -574215, 16096535, 4705056, 17713168 ], + [ -235790, -456383, 2076480, 14126013 ], + [ -145029, 1373099, 1147776, 7226208 ], [ -51826, 38850, 2915550, 71269 ], + [ 87, -452, 1800, 44 ], [ 23349, -103132, 738675, -163130 ], + [ 27853, -108309, -630630, 351252 ], + [ 1403725, 643437067, -190478253, -47332395 ], + [ 13885273, -1477537, -280945070, -48335100 ] ] gap> Polymake(poly,"FAR_HYPERPLANE"); [ 1, 0, 0, 0 ] gap> Polymake(poly,"FEASIBLE"); @@ -127,15 +155,21 @@ gap> Polymake(poly,"VERTICES"); [ [ 1/4, 1/75, 1/22 ], [ 1/37, 1/62, 1/19 ], [ 1/91, 1/24, 1/88 ], [ 1/59, 1/67, 1/5 ], [ 1/85, 1/31, 1/76 ], [ 1/4, 1/72, 1/44 ], [ 1/79, 1/21, 1/99 ], [ 1/3, 1/30, 1/12 ], [ 1/71, 1/48, 1/71 ] ] -gap> Polymake(poly,"VERTICES_IN_FACETS"); -[ [ 1, 6, 8 ], [ 2, 4, 9 ], [ 3, 4, 7 ], [ 6, 7, 8 ], [ 4, 7, 8 ], - [ 3, 7, 9 ], [ 6, 7, 9 ], [ 1, 4, 8 ], [ 1, 2, 4 ], [ 2, 6, 9 ], - [ 1, 2, 6 ], [ 3, 5, 9 ], [ 4, 5, 9 ], [ 3, 4, 5 ] ] +gap> Set(Polymake(poly,"VERTICES_IN_FACETS")); +[ [ 1, 2, 4 ], [ 1, 2, 6 ], [ 1, 4, 8 ], [ 1, 6, 8 ], [ 2, 4, 9 ], + [ 2, 6, 9 ], [ 3, 4, 5 ], [ 3, 4, 7 ], [ 3, 5, 9 ], [ 3, 7, 9 ], + [ 4, 5, 9 ], [ 4, 7, 8 ], [ 6, 7, 8 ], [ 6, 7, 9 ] ] ## test external files: +## polymake rewrites files in the old plain format in place, so work on a copy +## rather than on the one checked into the repository. +## gap> dir:=DirectoriesPackageLibrary("polymaking", "tst")[1];; +gap> tmpdir:=DirectoryTemporary();; +gap> FileString(Filename(tmpdir, "pplane.poly"), +> StringFile(Filename(dir, "pplane.poly")));; gap> olddatadir:=UserPreference("polymaking", "PolymakeDataDirectory");; -gap> SetUserPreference("polymaking", "PolymakeDataDirectory", Filename(dir, ""));; +gap> SetUserPreference("polymaking", "PolymakeDataDirectory", Filename(tmpdir, ""));; gap> plane:=CreatePolymakeObjectFromFile("pplane.poly"); diff --git a/tst/testall.g b/tst/testall.g index ace56ed..ad1e2db 100644 --- a/tst/testall.g +++ b/tst/testall.g @@ -1,4 +1,16 @@ LoadPackage("polymaking"); + +# Set POLYMAKING_CHULL to run the tests against a specific polymake convex hull +# backend, e.g. POLYMAKING_CHULL=cdd. Results must not depend on the choice. +if IsBound(GAPInfo.SystemEnvironment.POLYMAKING_CHULL) + and GAPInfo.SystemEnvironment.POLYMAKING_CHULL <> "" then + SetUserPreference("polymaking", "PolymakePreferences", + [ Concatenation("*.convex_hull ", + GAPInfo.SystemEnvironment.POLYMAKING_CHULL) ]); + Print("# using polymake convex hull backend: ", + GAPInfo.SystemEnvironment.POLYMAKING_CHULL, "\n"); +fi; + # This needs to be done manually exclude:=["visual.tst"]; TestDirectory( diff --git a/tst/userprefs.tst b/tst/userprefs.tst index e890467..5b7929b 100644 --- a/tst/userprefs.tst +++ b/tst/userprefs.tst @@ -60,13 +60,15 @@ gap> SetUserPreference("polymaking", "PolymakeCommand", "no/such/polymake");; gap> PolymakeCommand(); fail -# the polymake output preferences have sane defaults and are validated -gap> UserPreference("polymaking", "PolymakeQuiet"); +# the polymake output preferences are well-formed. Do not check them against +# their defaults: the test suite is run with PolymakePreferences set, to cover +# several convex hull backends. +gap> UserPreference("polymaking", "PolymakeQuiet") in [true, false]; +true +gap> IsString(UserPreference("polymaking", "PolymakeConfigPath")); +true +gap> ForAll(UserPreference("polymaking", "PolymakePreferences"), IsString); true -gap> UserPreference("polymaking", "PolymakeConfigPath"); -"" -gap> UserPreference("polymaking", "PolymakePreferences"); -[ ] # gap> SetUserPreference("polymaking", "PolymakeCommand", oldcmd);; diff --git a/tst/visual.tst b/tst/visual.tst index 4a334a3..6ec2f8c 100644 --- a/tst/visual.tst +++ b/tst/visual.tst @@ -1,8 +1,12 @@ -gap> LoadPackage("polymaking"); +# Excluded from tst/testall.g: VISUAL wants to open a viewer. +gap> START_TEST("visual.tst"); gap> dir:=DirectoriesPackageLibrary("polymaking", "tst")[1];; -gap> SetUserPreference("polymaking", "PolymakeDataDirectory", Filename(dir, ""));; -gap> poly := CreatePolymakeObjectFromFile(dir, "visual.poly"); +gap> tmpdir:=DirectoryTemporary();; +gap> FileString(Filename(tmpdir, "visual.poly"), +> StringFile(Filename(dir, "visual.poly")));; +gap> SetUserPreference("polymaking", "PolymakeDataDirectory", Filename(tmpdir, ""));; +gap> poly := CreatePolymakeObjectFromFile(tmpdir, "visual.poly"); gap> Polymake(poly, "VISUAL"); -fail - +fail +gap> STOP_TEST("visual.tst", 1); From bdb0ad7947a115772836128a3366afbccef7e894 Mon Sep 17 00:00:00 2001 From: Max Horn Date: Sun, 16 Aug 2026 22:40:04 +0200 Subject: [PATCH 2/3] Avoid the two argument form of Set in the tests It was only declared in GAP 4.11, and polymaking still supports older versions. Co-Authored-By: Claude Opus 5 --- tst/polymaking.tst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tst/polymaking.tst b/tst/polymaking.tst index c98702c..7c406a4 100644 --- a/tst/polymaking.tst +++ b/tst/polymaking.tst @@ -101,7 +101,7 @@ gap> Collected(List(adj, Length)); [ 7, 1 ], [ 9, 1 ] ] gap> Polymake(poly,"FACET_DEGREES"); [ 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3 ] -gap> Set(Polymake(poly,"FACETS"), NormalizeRow); +gap> Set(List(Polymake(poly,"FACETS"), NormalizeRow)); [ [ -2303641863, 164204234285, 12897123192, -3359875640 ], [ -1580061339, 1195816838, 93208905900, 843077785 ], [ -519350247, 28577972485, 5781969216, -256601080 ], From c02c5f9190eed5b1bbdd6d658957729ca7c265dd Mon Sep 17 00:00:00 2001 From: Max Horn Date: Sun, 16 Aug 2026 23:53:29 +0200 Subject: [PATCH 3/3] Apply suggestion from @fingolfin --- tst/polymaking.tst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tst/polymaking.tst b/tst/polymaking.tst index 7c406a4..dafdbdd 100644 --- a/tst/polymaking.tst +++ b/tst/polymaking.tst @@ -15,7 +15,7 @@ gap> NormalizeRow := function(v) gap> CanonicalFaceList := function(L) > local c; > c := ShallowCopy(L); -> Sort(c, function(a, b) return [Size(a), a] < [Size(b), b]; end); +> SortBy(c, a -> [Size(a), a]); > return c; > end;; gap> CoversOfFaceList := function(faces)