Merge pull request #3253 from mahrud/refactor/monideal

Refactoring monideal.m2

M2/Macaulay2/m2/matrix1.m2

@@ -13,7 +13,7 @@ notsamering := (X,Y) -> (
 nottosamering := (X,Y) -> (
      if X === Y then error("expected ",pluralsynonym X, " for compatible rings")
      else error("expected ",X.synonym," and ",Y.synonym," for compatible rings"))
-samering := (M,N) -> if ring M === ring N then (M,N) else notsamering(class M,class N)
+samering = (M,N) -> if ring M === ring N then (M,N) else notsamering(class M,class N)
 tosamering := (M,N) -> if ring M === ring N then (M,N) else (
      z := try 0_(ring M) + 0_(ring N) else nottosamering(class M,class N);
      (promote(M,ring z),promote(N,ring z)))
@@ -555,11 +555,9 @@ ideal Matrix := Ideal => (f) -> (
 	  );
      new Ideal from { symbol generators => f, symbol ring => R, symbol cache => new CacheTable } )
 
-ideal Module := Ideal => (M) -> (
-     F := ambient M;
-     if isSubmodule M and rank F === 1 then ideal generators M
-     else error "expected a submodule of a free module of rank 1"
-     )
+ideal Module := Ideal => M -> if isIdeal M then ideal generators M else (
+    error "expected a submodule of a free module of rank 1")
+
 idealPrepare = method()
 idealPrepare RingElement := 
 idealPrepare Number := identity

M2/Macaulay2/m2/matrix2.m2

@@ -135,14 +135,16 @@ complement Matrix := Matrix => (f) -> (
      else if instance(R,QuotientRing) then map(target f,,R ** complement lift(f,ambient R))
      else error "complement: expected matrix over affine ring or finitely generated ZZ-algebra")
 
+-----------------------------------------------------------------------------
+-- mingens and trim
+-----------------------------------------------------------------------------
+
 -- the method is declared in gb.m2
+-- TODO: the strategies should be separated
 mingens Ideal  := Matrix => opts -> I -> mingens(module I, opts)
-mingens Module := Matrix => opts -> (cacheValue symbol mingens) ((M) -> (
-        c := runHooks((mingens, Module), (opts, M));
-        if c =!= null then c else error "mingens: no method implemented for this type of module"))
-
--- FIXME: This is kind of a hack. The strategies should be separated in mingensHelper
-mingensHelper = ((opts, M) -> (
+mingens Module := Matrix => opts -> M -> if isFreeModule M then generators M else cacheHooks(
+    symbol mingens, M, (mingens, Module), (opts, M), (opts, M) -> (
+	if opts.Strategy === null then opts = opts ++ { Strategy => Complement };
  	  mingb := m -> gb (m, StopWithMinimalGenerators=>true, Syzygies=>false, ChangeMatrix=>false);
 	  zr := f -> if f === null or f == 0 then null else f;
 	  F := ambient M;
@@ -164,25 +166,15 @@ mingensHelper = ((opts, M) -> (
 		    else mingens mingb (id_F % mingb(M.relations)))
 	       else id_F)))
 
-addHook((mingens, Module), Strategy => Inhomogeneous, (opts, M) -> mingensHelper(opts ++ {Strategy => Inhomogeneous}, M))
-addHook((mingens, Module), Strategy => Complement,    (opts, M) -> mingensHelper(opts ++ {Strategy => Complement},    M))
-
 trim = method (Options => { Strategy => null -* TODO: add DegreeLimit => {} *-})
-trim Ring := Ring => opts -> (R) -> R
-trim QuotientRing := opts -> (R) -> (
-     f := presentation R;
-     A := ring f;
-     A/(trim(ideal f,opts)))
-
--- TODO: why is the caching key an Option?
-trim Ideal  := Ideal  => opts -> (cacheValue (symbol trim => opts)) ((I) -> ideal trim(module I, opts))
-trim Module := Module => opts -> (cacheValue symbol trim) (M -> (
-	if isFreeModule M then return M;
-	c := runHooks((trim, Module), (opts, M));
-	if c =!= null then c else error "trim: no method implemented for this type of module"))
-
--- FIXME: This is kind of a hack. The strategies should be separated in trimHelper
-trimHelper = ((opts, M) -> (
+trim Ring         := Ring => o -> identity
+trim QuotientRing := Ring => o -> R -> quotient trim(ideal presentation R, o)
+
+-- TODO: the strategies should be separated
+trim Ideal  := Ideal  => opts -> I -> ideal trim(module I, opts)
+trim Module := Module => opts -> M -> if isFreeModule M then M else cacheHooks(
+    (symbol trim, opts), M, (trim, Module), (opts, M), (opts, M) -> (
+	if opts.Strategy === null then opts = opts ++ { Strategy => Complement };
 	  -- we preserve the ambient free module of which M is subquotient and try to minimize the generators and relations
 	  --   without computing an entire gb
 	  -- does using "complement" as in "mingens Module" above offer a benefit?
@@ -244,9 +236,6 @@ trimHelper = ((opts, M) -> (
 	  N.cache.trim = N;
 	  N))
 
-addHook((trim, Module), Strategy => Inhomogeneous, (opts, M) -> trimHelper(opts ++ {Strategy => Inhomogeneous}, M))
-addHook((trim, Module), Strategy => Complement,    (opts, M) -> trimHelper(opts ++ {Strategy => Complement},    M))
-
 trimPID := M -> if M.?relations then (if M.?generators then trimPID image generators M else ambient M) / trimPID image relations M else if not M.?generators then M else (
     f := presentation M;
     (g,ch) := smithNormalForm(f, ChangeMatrix => {true, false});
@@ -266,6 +255,7 @@ addHook((trim, Module), Strategy => "PID",
 	)
     )
 
+-----------------------------------------------------------------------------
 
 syz Matrix := Matrix => opts -> (f) -> (
     c := runHooks((syz, Matrix), (opts, f));