semifp: Add a method for IsomorphismFpSemigroup #426

ChristopherRussell · 2017-12-15T12:40:21Z

This method applies to factorizable inverse monoids of partial perms. Method is
based on the paper Presentations of Factorizable Inverse Monoids by D. Easdown,
J. East, and D. G. FitzGerald (2004).

ChristopherRussell · 2017-12-15T12:44:20Z

I'm not sure if I have done lines 632 - 640 of semifp.gi in the best way. Also BruteForceInverseCheck typically (for example, on my tests) takes much longer than BruteForceIsoCheck - does this say that the way I have created the function inv is ineffective?

MF := F / rels; # FpSemigroup which is isomorphic to M, with different gens.
fam := ElementsFamily(FamilyObj(MF));
T := Semigroup(Concatenation(SS, GG)); # M with isomorphic generators to MF

map := x -> ElementOfFpSemigroup(fam, EvaluateWord(GeneratorsOfSemigroup(F),
                                                   Factorization(T, x)));
inv := x -> EvaluateWord(GeneratorsOfSemigroup(T),
        SEMIGROUPS.ExtRepObjToWord(ExtRepOfObj(x)));

  return MagmaIsomorphismByFunctionsNC(M, MF, map, inv);
end);

ChristopherRussell · 2017-12-15T15:23:14Z

@james-d-mitchell I'm not sure why the travis tests are failing. The issues seem unrelated to the new code in this PR.

james-d-mitchell · 2017-12-18T10:19:09Z

Hmm, @ChristopherRussell I'm not so sure, do the test all pass on your computer?

wilfwilson · 2017-12-18T13:54:21Z

@ChristopherRussell if you rebase on the most recent master branch, the diffs should disappear.

ChristopherRussell · 2017-12-18T14:05:31Z

Thanks @wilfwilson!

ChristopherRussell · 2017-12-21T15:11:48Z

I have fixed some of the issues in sempfp.tst (made a fix to IsomorphismFpMonoid and stopped calling BrandtSemigroup which is not yet in stable-3.0).

@james-d-mitchell I can't reproduce the remaining errors in the failed travis test, which only occur in one of the builds, and I was hoping you could help.

Also I am am going to write a function EvaluateExtRep to save converting ExtReps to words then using EvaluateWord. The function would look something like this:

InstallMethod(EvaluateExtRep,
"for a multiplicative element collection and a list of positive ints of even length",
[IsMultiplicativeElementCollection, IsList],
function(gens, w)
  local res, i;
    if Length(w) = 0 then
      return One(gens); # need to account for semigroup not being a moniod?
    elif Length(w) mod 2 = 1 then
      ErrorNoReturn("Semigroups: EvaluateExtRep, the second argument must be ",
                     "a list of even length");
    fi;
    res := gens[AbsInt(w[1])] ^ w[2];
    for i in [3, 5 .. Length(w) - 1] do
      res := res * gens[AbsInt(w[i])] ^ w[i + 1];
    od;
    return res;
end);

We also noticed that EvaluateWord has several implementations in semigroups/gap/main/orbits.gi and you asked me to remind you that these should perhaps be replaced by a single method with filter IsMultiplicativeElementColl. The three methods in orbits.gi are:

InstallMethod(EvaluateWord, "for bipartition coll and list of integers",
[IsBipartitionCollection, IsList],
function(gens, w)
    local i, res;
    if Length(w) = 0  then
      return One(gens);
    fi;
    res := gens[AbsInt(w[1])] ^ SignInt(w[1]);
    for i in [2 .. Length(w)]  do
      res := res * gens[AbsInt(w[i])] ^ SignInt(w[i]);
    od;
    return res;
end);

InstallMethod(EvaluateWord, "for partial perm coll and list of integers",
[IsPartialPermCollection, IsList],
function(gens, w)
    local i, res;
    if Length(w) = 0  then
      return One(gens);
    fi;
    res := gens[AbsInt(w[1])] ^ SignInt(w[1]);
    for i  in [2 .. Length(w)]  do
      res := res * gens[AbsInt(w[i])] ^ SignInt(w[i]);
    od;
    return res;
end);

InstallMethod(EvaluateWord,
"for Rees 0-matrix semigroup element collection and a list of positive ints",
[IsReesZeroMatrixSemigroupElementCollection, IsList],
function(gens, w)
    local i, res;
    if Length(w) = 0  then
      return SEMIGROUPS.UniversalFakeOne;
    fi;
    res := gens[w[1]];
    for i in [2 .. Length(w)]  do
      res := res * gens[w[i]];
    od;
    return res;
end);

james-d-mitchell · 2018-01-16T10:47:23Z

@ChristopherRussell Maybe the failure here is related to Issue #435? I also can't reproduce this, on my computer. Can you rebase on the top of stable-3.0 (which now contains some changes not present when travis ran) and see if the tests pass?

james-d-mitchell · 2018-01-16T10:53:07Z

Also I like your suggestion for EvaluateExtRep, I'd call it EvaluateExtRepObjWord or similar though. I'd handle the case of monoid separately (i.e. in another method), with the filter being IsMultiplicativeElementWithOneCollection and in the version for IsMultplicativeElementCollection I'd just not allow the case of words of length 0 (error).

Also I like the suggestion about combining the methods for EvaluateWord, I'd be tempted to have two methods again one for IsMultiplicativeElementCollection and another for IsMultiplicativeElementWithOneCollection they can both use the method for bipartitions.

This method applies to factorizable inverse monoids of partial perms. Method is based on the paper Presentations of Factorizable Inverse Monoids by D. Easdown, J. East, and D. G. FitzGerald (2004).

ChristopherRussell · 2018-01-16T12:28:27Z

I will make the EvaluateExtRepObjWord and EvaluateWord changes with PR #443

james-d-mitchell · 2018-01-16T13:51:56Z

This looks good. Here are a couple of minor points:

You mention a bug in factorisation in a comment, what's this bug can you file and issue?
Can you add an extreme test together with a comment that your method is quicker than the default one in that case?
Can you please reference the paper as a comment before the method?

james-d-mitchell

Looks good other than the minor comments I've made.

james-d-mitchell · 2018-01-16T13:52:48Z

gap/semigroups/semifp.gi

+      list[i] := list[i] + s;
+    od;
+    return list;
+  end;


Couldn't this whole method just be list{[1, 3 .. Length(list) - 1]} := list{[1, 3 .. Length(list) - 1]} + s?

Yes, changed.

james-d-mitchell · 2018-01-16T13:53:13Z

gap/semigroups/semifp.gi

+  end;
+
+  # FIXME: this shouldn't be necessary but for an error with
+  # MinimalFactorization


Please file an issue for this error, or is this already resolved?

I can't remember the issue now. It was something we noticed when writing code together and it hasn't been resolved. However I am not sure if it was a bug or just that the format of the output didn't suit our purposes (e.g. due to how it dealt with inverses or the identity when factorizing).

My tests still pass when I remove the rec(acting := false) so that didn't give any hints.

Aha, then this was fixed, it was Issue #424. Please remove the comment.

Ah okay, great.

Please also remove the unnecessary code.

Yes, sorry. Done it now.

james-d-mitchell · 2018-01-16T13:54:06Z

gap/semigroups/semifp.gi

+  map := InverseGeneralMapping(IsomorphismPermGroup(G));
+  H   := Source(map);
+  o   := Enumerate(LambdaOrb(M));
+  #R_tilde


Can we have a line/page reference in the paper for this?

james-d-mitchell · 2018-01-16T13:55:32Z

gap/semigroups/semifp.gi

+  map := x -> ElementOfFpSemigroup(fam, EvaluateWord(GeneratorsOfSemigroup(F),
+                                                     Factorization(T, x)));
+  inv := x -> EvaluateWord(GeneratorsOfSemigroup(T),
+         SEMIGROUPS.ExtRepObjToWord(ExtRepOfObj(x)));


This should be indented so that it is aligned with the ( in (GeneratorsOf...

ChristopherRussell force-pushed the fp branch from ca8efcb to a069a4c Compare December 15, 2017 14:14

james-d-mitchell added 3.0 new-feature A label for PRs that contain new features labels Dec 18, 2017

ChristopherRussell force-pushed the fp branch from a069a4c to 18a77c9 Compare December 18, 2017 14:03

ChristopherRussell force-pushed the fp branch from 18a77c9 to 072102c Compare December 20, 2017 14:12

ChristopherRussell changed the base branch from master to stable-3.0 December 20, 2017 15:38

ChristopherRussell force-pushed the fp branch 4 times, most recently from da0d313 to 29e8161 Compare December 21, 2017 13:48

semifp: Add a method for IsomorphismFpSemigroup

f2b555a

This method applies to factorizable inverse monoids of partial perms. Method is based on the paper Presentations of Factorizable Inverse Monoids by D. Easdown, J. East, and D. G. FitzGerald (2004).

ChristopherRussell force-pushed the fp branch from 29e8161 to 72d2478 Compare January 16, 2018 11:08

james-d-mitchell requested changes Jan 16, 2018

View reviewed changes

ChristopherRussell force-pushed the fp branch 2 times, most recently from 908d5ae to 35f5fda Compare January 16, 2018 15:39

semifp: fix to IsomorphismFpMonoid

c978387

ChristopherRussell force-pushed the fp branch from 35f5fda to c978387 Compare January 16, 2018 16:25

james-d-mitchell approved these changes Jan 17, 2018

View reviewed changes

james-d-mitchell merged commit 0bb1e13 into semigroups:stable-3.0 Jan 17, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

semifp: Add a method for IsomorphismFpSemigroup #426

semifp: Add a method for IsomorphismFpSemigroup #426

ChristopherRussell commented Dec 15, 2017

ChristopherRussell commented Dec 15, 2017 •

edited

Loading

ChristopherRussell commented Dec 15, 2017 •

edited

Loading

james-d-mitchell commented Dec 18, 2017

wilfwilson commented Dec 18, 2017

ChristopherRussell commented Dec 18, 2017

ChristopherRussell commented Dec 21, 2017 •

edited

Loading

james-d-mitchell commented Jan 16, 2018

james-d-mitchell commented Jan 16, 2018

ChristopherRussell commented Jan 16, 2018

james-d-mitchell commented Jan 16, 2018

james-d-mitchell left a comment

james-d-mitchell Jan 16, 2018

ChristopherRussell Jan 16, 2018

james-d-mitchell Jan 16, 2018

ChristopherRussell Jan 16, 2018 •

edited

Loading

james-d-mitchell Jan 16, 2018

ChristopherRussell Jan 16, 2018

james-d-mitchell Jan 16, 2018

ChristopherRussell Jan 16, 2018

james-d-mitchell Jan 16, 2018

james-d-mitchell Jan 16, 2018

semifp: Add a method for IsomorphismFpSemigroup #426

semifp: Add a method for IsomorphismFpSemigroup #426

Conversation

ChristopherRussell commented Dec 15, 2017

ChristopherRussell commented Dec 15, 2017 • edited Loading

ChristopherRussell commented Dec 15, 2017 • edited Loading

james-d-mitchell commented Dec 18, 2017

wilfwilson commented Dec 18, 2017

ChristopherRussell commented Dec 18, 2017

ChristopherRussell commented Dec 21, 2017 • edited Loading

james-d-mitchell commented Jan 16, 2018

james-d-mitchell commented Jan 16, 2018

ChristopherRussell commented Jan 16, 2018

james-d-mitchell commented Jan 16, 2018

james-d-mitchell left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ChristopherRussell Jan 16, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ChristopherRussell commented Dec 15, 2017 •

edited

Loading

ChristopherRussell commented Dec 15, 2017 •

edited

Loading

ChristopherRussell commented Dec 21, 2017 •

edited

Loading

ChristopherRussell Jan 16, 2018 •

edited

Loading