From 948667b45c8a61bc62c2906109c1298d10745858 Mon Sep 17 00:00:00 2001 From: Kamil Zabielski <50334623+limakzi@users.noreply.github.com> Date: Wed, 11 Mar 2026 18:33:24 +0100 Subject: [PATCH 1/8] fix: Fix IsSubset for cyclotomic domains --- lib/cyclotom.gi | 25 ++++++++++++++++++++++ tst/testinstall/cyclotom.tst | 40 ++++++++++++++++++++++++++++++++++++ 2 files changed, 65 insertions(+) diff --git a/lib/cyclotom.gi b/lib/cyclotom.gi index 5aeefe77fe..2619812f4e 100644 --- a/lib/cyclotom.gi +++ b/lib/cyclotom.gi @@ -2065,6 +2065,31 @@ function (A,B) end ); +InstallMethod( IsSubset, "for a cyclotomic semiring and a range", + [IsCyclotomicCollection and IsSemiringWithOne, + IsRange], +function (D, R) + local lo; + if IsEmpty( R ) then return true; fi; + # Ranges contain only integers. + if IsPositiveIntegers( D ) then + # PositiveIntegers contains all integers >= 1 + lo := Minimum( R[1], R[Length(R)] ); + return lo >= 1; + elif IsNonnegativeIntegers( D ) then + # NonnegativeIntegers contains all integers >= 0 + lo := Minimum( R[1], R[Length(R)] ); + return lo >= 0; + elif IsIntegers( D ) or IsRationals( D ) + or IsGaussianIntegers( D ) or IsGaussianRationals( D ) + or (HasIsWholeFamily( D ) and IsWholeFamily( D )) then + # These domains contain all integers, so any range is a subset + return true; + fi; + TryNextMethod(); +end ); + + InstallMethod( Intersection2, "for certain cyclotomic semirings", [IsCyclotomicCollection and IsSemiringWithOne, IsCyclotomicCollection and IsSemiringWithOne], diff --git a/tst/testinstall/cyclotom.tst b/tst/testinstall/cyclotom.tst index 3763995c43..7a8b2dad09 100644 --- a/tst/testinstall/cyclotom.tst +++ b/tst/testinstall/cyclotom.tst @@ -352,5 +352,45 @@ gap> SetX(r, r, {i,j} -> IsSubset(sets[i],sets[j]) = (i>=j)); gap> SetX(r, r, {i,j} -> (sets[i]=sets[j]) = (i=j)); [ true ] +# +# IsSubset for cyclotomic semirings and ranges +# +gap> IsSubset(Integers, [1..10^15]); +true +gap> IsSubset(Integers, [-10..10]); +true +gap> IsSubset(Rationals, [1..10^15]); +true +gap> IsSubset(GaussianIntegers, [1..10^15]); +true +gap> IsSubset(GaussianRationals, [1..10^15]); +true +gap> IsSubset(PositiveIntegers, [1..10^15]); +true +gap> IsSubset(PositiveIntegers, [0..10]); +false +gap> IsSubset(PositiveIntegers, [-5..5]); +false +gap> IsSubset(NonnegativeIntegers, [0..10^15]); +true +gap> IsSubset(NonnegativeIntegers, [-1..10]); +false +gap> IsSubset(NonnegativeIntegers, [1..10]); +true + +# descending ranges +gap> IsSubset(PositiveIntegers, [10,9..1]); +true +gap> IsSubset(PositiveIntegers, [10,9..0]); +false +gap> IsSubset(NonnegativeIntegers, [10,9..0]); +true +gap> IsSubset(NonnegativeIntegers, [5,3..-1]); +false + +# empty ranges +gap> IsSubset(PositiveIntegers, [1..0]); +true + # gap> STOP_TEST("cyclotom.tst"); From 6cf0efb49d650a7c19682b96d811e3e8a59bfa6a Mon Sep 17 00:00:00 2001 From: Kamil Zabielski <50334623+limakzi@users.noreply.github.com> Date: Thu, 12 Mar 2026 21:53:12 +0100 Subject: [PATCH 2/8] Fix IsSubset implementation --- lib/cyclotom.gi | 25 +++++++------------------ 1 file changed, 7 insertions(+), 18 deletions(-) diff --git a/lib/cyclotom.gi b/lib/cyclotom.gi index 2619812f4e..5833d4a33a 100644 --- a/lib/cyclotom.gi +++ b/lib/cyclotom.gi @@ -2069,24 +2069,13 @@ InstallMethod( IsSubset, "for a cyclotomic semiring and a range", [IsCyclotomicCollection and IsSemiringWithOne, IsRange], function (D, R) - local lo; - if IsEmpty( R ) then return true; fi; - # Ranges contain only integers. - if IsPositiveIntegers( D ) then - # PositiveIntegers contains all integers >= 1 - lo := Minimum( R[1], R[Length(R)] ); - return lo >= 1; - elif IsNonnegativeIntegers( D ) then - # NonnegativeIntegers contains all integers >= 0 - lo := Minimum( R[1], R[Length(R)] ); - return lo >= 0; - elif IsIntegers( D ) or IsRationals( D ) - or IsGaussianIntegers( D ) or IsGaussianRationals( D ) - or (HasIsWholeFamily( D ) and IsWholeFamily( D )) then - # These domains contain all integers, so any range is a subset - return true; - fi; - TryNextMethod(); + if IsEmpty(R) then return true; fi; + # Since D is a semiring-with-one, and thus in particular an additive magma, + # it suffices to check whether the start and end point of the given range + # are contained in it. Actually it suffices if the smaller one is contained + # in it, but figuring out which end is smaller is more work than just + # directly checking both end points. + return First(R) in D and Last(R) in D; end ); From 0b71ca195852a84fba752b91702ce0fedaa8523a Mon Sep 17 00:00:00 2001 From: Kamil Zabielski <50334623+limakzi@users.noreply.github.com> Date: Thu, 12 Mar 2026 22:35:56 +0100 Subject: [PATCH 3/8] Simplify tests for IsSubset --- tst/testinstall/cyclotom.tst | 47 +++++++----------------------------- 1 file changed, 9 insertions(+), 38 deletions(-) diff --git a/tst/testinstall/cyclotom.tst b/tst/testinstall/cyclotom.tst index 7a8b2dad09..bd6e7d3803 100644 --- a/tst/testinstall/cyclotom.tst +++ b/tst/testinstall/cyclotom.tst @@ -1,4 +1,4 @@ -#@local a,cyc,gm,i,l1,l2,l3,mat,n,r,x,y,z,sets +#@local a,b,cyc,gm,i,l1,l2,l3,mat,n,r,ranges,s,x,y,z,sets gap> START_TEST("cyclotom.tst"); # Check basic arithmetic operations. @@ -353,44 +353,15 @@ gap> SetX(r, r, {i,j} -> (sets[i]=sets[j]) = (i=j)); [ true ] # -# IsSubset for cyclotomic semirings and ranges +# IsSubset for cyclotomic semirings and ranges for large bounds # -gap> IsSubset(Integers, [1..10^15]); -true -gap> IsSubset(Integers, [-10..10]); -true -gap> IsSubset(Rationals, [1..10^15]); -true -gap> IsSubset(GaussianIntegers, [1..10^15]); -true -gap> IsSubset(GaussianRationals, [1..10^15]); -true -gap> IsSubset(PositiveIntegers, [1..10^15]); -true -gap> IsSubset(PositiveIntegers, [0..10]); -false -gap> IsSubset(PositiveIntegers, [-5..5]); -false -gap> IsSubset(NonnegativeIntegers, [0..10^15]); -true -gap> IsSubset(NonnegativeIntegers, [-1..10]); -false -gap> IsSubset(NonnegativeIntegers, [1..10]); -true - -# descending ranges -gap> IsSubset(PositiveIntegers, [10,9..1]); -true -gap> IsSubset(PositiveIntegers, [10,9..0]); -false -gap> IsSubset(NonnegativeIntegers, [10,9..0]); -true -gap> IsSubset(NonnegativeIntegers, [5,3..-1]); -false - -# empty ranges -gap> IsSubset(PositiveIntegers, [1..0]); -true +gap> sets:=[ PositiveIntegers, NonnegativeIntegers, Integers, GaussianIntegers, GaussianRationals, Cyclotomics ];; +gap> b:=2^(8*GAPInfo.BytesPerVariable - 6);; +gap> ranges:=[ [-2..-1], [-1..0], [-1..1], [0..1], [1..2], [-b..-1], [-b..0], [-b..1], [-1..b], [0..b], [1..b], [-b,b]];; +gap> SetX(ranges, sets, {r,s} -> IsSubset(s,r) = (IsEmpty(r) or (First(r) in s and Last(r) in s))); +[ true ] +gap> SetX(ranges, sets, {r,s} -> not IsSubset(r,s)); +[ true ] # gap> STOP_TEST("cyclotom.tst"); From 95febd769f695eaed40e10ffaf2e2945b988dde2 Mon Sep 17 00:00:00 2001 From: Kamil Zabielski <50334623+limakzi@users.noreply.github.com> Date: Sun, 15 Mar 2026 09:58:14 +0100 Subject: [PATCH 4/8] Add IsSubset method for a range and a cyclotomic --- lib/cyclotom.gi | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/lib/cyclotom.gi b/lib/cyclotom.gi index 5833d4a33a..2185e986a6 100644 --- a/lib/cyclotom.gi +++ b/lib/cyclotom.gi @@ -2079,6 +2079,12 @@ function (D, R) end ); +InstallMethod( IsSubset, "for a range and a cyclotomic semiring", + [IsRange, + IsCyclotomicCollection and IsSemiringWithOne], + ReturnFalse ); + + InstallMethod( Intersection2, "for certain cyclotomic semirings", [IsCyclotomicCollection and IsSemiringWithOne, IsCyclotomicCollection and IsSemiringWithOne], From d99725436b688a2756aac9995e7454ee691490d6 Mon Sep 17 00:00:00 2001 From: Kamil Zabielski <50334623+limakzi@users.noreply.github.com> Date: Sun, 15 Mar 2026 10:14:34 +0100 Subject: [PATCH 5/8] Add IsSubset method for a list and a cyclotomic semiring --- lib/cyclotom.gi | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/cyclotom.gi b/lib/cyclotom.gi index 2185e986a6..1e0a8236e8 100644 --- a/lib/cyclotom.gi +++ b/lib/cyclotom.gi @@ -2079,8 +2079,8 @@ function (D, R) end ); -InstallMethod( IsSubset, "for a range and a cyclotomic semiring", - [IsRange, +InstallMethod( IsSubset, "for a list and a cyclotomic semiring", + [IsList, IsCyclotomicCollection and IsSemiringWithOne], ReturnFalse ); From f1ec0cd9b3cd4aaa78105746f957327afb4ea9ee Mon Sep 17 00:00:00 2001 From: Max Horn Date: Mon, 23 Mar 2026 12:14:16 +0100 Subject: [PATCH 6/8] Apply suggestions from code review Co-authored-by: Max Horn --- lib/cyclotom.gi | 4 ++-- tst/testinstall/cyclotom.tst | 9 ++++++++- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/lib/cyclotom.gi b/lib/cyclotom.gi index 1e0a8236e8..427822cbca 100644 --- a/lib/cyclotom.gi +++ b/lib/cyclotom.gi @@ -2079,8 +2079,8 @@ function (D, R) end ); -InstallMethod( IsSubset, "for a list and a cyclotomic semiring", - [IsList, +InstallMethod( IsSubset, "for a finite list and a cyclotomic semiring", + [IsList and IsFinite, IsCyclotomicCollection and IsSemiringWithOne], ReturnFalse ); diff --git a/tst/testinstall/cyclotom.tst b/tst/testinstall/cyclotom.tst index bd6e7d3803..86071fbe6d 100644 --- a/tst/testinstall/cyclotom.tst +++ b/tst/testinstall/cyclotom.tst @@ -357,11 +357,18 @@ gap> SetX(r, r, {i,j} -> (sets[i]=sets[j]) = (i=j)); # gap> sets:=[ PositiveIntegers, NonnegativeIntegers, Integers, GaussianIntegers, GaussianRationals, Cyclotomics ];; gap> b:=2^(8*GAPInfo.BytesPerVariable - 6);; -gap> ranges:=[ [-2..-1], [-1..0], [-1..1], [0..1], [1..2], [-b..-1], [-b..0], [-b..1], [-1..b], [0..b], [1..b], [-b,b]];; +gap> ranges:=[ [-2..-1], [-1..0], [-1..1], [0..1], [1..2], [-b..-1], [-b..0], [-b..1], [-1..b], [0..b], [1..b], [-b..b]];; gap> SetX(ranges, sets, {r,s} -> IsSubset(s,r) = (IsEmpty(r) or (First(r) in s and Last(r) in s))); [ true ] gap> SetX(ranges, sets, {r,s} -> not IsSubset(r,s)); [ true ] # +gap> lists:=[ [-2,-1], [], [-1,0,1], [0,1], [1,2], [-b,-1], [-b,0], [-b,1], [-1,b], [0,b], [1,b], [-b,b]];; + +# +gap> SetX(ranges, lists, {r,s} -> IsSubset(s,r) = (IsEmpty(r) or (First(r) in s and Last(r) in s))); +[ true ] +gap> SetX(ranges, lists, {r,s} -> not IsSubset(r,s)); +[ true ] gap> STOP_TEST("cyclotom.tst"); From 8dcc2cdbd2ce4fff312926ea3f52bd3a518468cb Mon Sep 17 00:00:00 2001 From: Max Horn Date: Mon, 23 Mar 2026 12:16:46 +0100 Subject: [PATCH 7/8] Apply suggestion from @fingolfin --- tst/testinstall/cyclotom.tst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tst/testinstall/cyclotom.tst b/tst/testinstall/cyclotom.tst index 86071fbe6d..59034107ec 100644 --- a/tst/testinstall/cyclotom.tst +++ b/tst/testinstall/cyclotom.tst @@ -367,8 +367,8 @@ gap> SetX(ranges, sets, {r,s} -> not IsSubset(r,s)); gap> lists:=[ [-2,-1], [], [-1,0,1], [0,1], [1,2], [-b,-1], [-b,0], [-b,1], [-1,b], [0,b], [1,b], [-b,b]];; # -gap> SetX(ranges, lists, {r,s} -> IsSubset(s,r) = (IsEmpty(r) or (First(r) in s and Last(r) in s))); +gap> SetX(lists, sets, {l,s} -> IsSubset(s,l) = ForAll(l, x -> x in s))); [ true ] -gap> SetX(ranges, lists, {r,s} -> not IsSubset(r,s)); +gap> SetX(lists, sets, {l,s} -> not IsSubset(l,s)); [ true ] gap> STOP_TEST("cyclotom.tst"); From 685a869cd12c82f2411774b1a1a1fed1e0115fc5 Mon Sep 17 00:00:00 2001 From: Max Horn Date: Mon, 23 Mar 2026 12:17:55 +0100 Subject: [PATCH 8/8] sigh --- tst/testinstall/cyclotom.tst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tst/testinstall/cyclotom.tst b/tst/testinstall/cyclotom.tst index 59034107ec..246b0d7cad 100644 --- a/tst/testinstall/cyclotom.tst +++ b/tst/testinstall/cyclotom.tst @@ -1,4 +1,4 @@ -#@local a,b,cyc,gm,i,l1,l2,l3,mat,n,r,ranges,s,x,y,z,sets +#@local a,b,cyc,gm,i,l1,l2,l3,mat,n,r,ranges,s,x,y,z,sets,lists gap> START_TEST("cyclotom.tst"); # Check basic arithmetic operations. @@ -367,7 +367,7 @@ gap> SetX(ranges, sets, {r,s} -> not IsSubset(r,s)); gap> lists:=[ [-2,-1], [], [-1,0,1], [0,1], [1,2], [-b,-1], [-b,0], [-b,1], [-1,b], [0,b], [1,b], [-b,b]];; # -gap> SetX(lists, sets, {l,s} -> IsSubset(s,l) = ForAll(l, x -> x in s))); +gap> SetX(lists, sets, {l,s} -> IsSubset(s,l) = ForAll(l, x -> x in s)); [ true ] gap> SetX(lists, sets, {l,s} -> not IsSubset(l,s)); [ true ]