Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Error in DirectFactorsFinder #316

Open
fingolfin opened this issue Apr 8, 2022 · 1 comment
Open

Error in DirectFactorsFinder #316

fingolfin opened this issue Apr 8, 2022 · 1 comment
Labels
bug Any bug should have this label, even if it also has a more generic label

Comments

@fingolfin
Copy link
Member

During tests in https://github.com/gap-packages/ClassicalMaximals I've run into an error in RECOG.DirectFactorsFinder. It is a pretty obvious bug which I think this patch should fix; I made the context of the patch big enough to allow seeing the problem in the code (fgens[3] was not defined before; it only works most of the time because it aborts before ever getting to the code accessing it).

diff --git a/gap/projective/d247.gi b/gap/projective/d247.gi
index 7f82f56..00080ea 100644
--- a/gap/projective/d247.gi
+++ b/gap/projective/d247.gi
@@ -93,12 +93,13 @@ end;
 RECOG.DirectFactorsFinder := function(gens,facgens,k,eq)
   local equal,fgens,i,j,l,o,pgens,pr,z;
   fgens := [];
   pr := ProductReplacer(facgens);
   Add(fgens,Next(pr));
   Add(fgens,Next(pr));
+  Add(fgens,Next(pr));
   if eq(fgens[1]*fgens[2],fgens[2]*fgens[1]) and
      eq(fgens[1]*fgens[3],fgens[3]*fgens[1]) then
       if eq(fgens[2]*fgens[3],fgens[3]*fgens[2]) then
           i := 0;
           while i <= 4 do
               Add(fgens,Next(pr),1);

Alas, I don't have time to properly vet this, or even add a test case, so I am just reporting it here for now

@fingolfin fingolfin added the bug Any bug should have this label, even if it also has a more generic label label Sep 22, 2022
@fingolfin
Copy link
Member Author

Also, this code is crap because it creates a completely new product replacer from scratch (which involves a lot of overhead) instead of reusing one from "somewhere" sigh

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Any bug should have this label, even if it also has a more generic label
Projects
None yet
Development

No branches or pull requests

1 participant