#5882: [with patch; needs work] implement general package for finitely generated
not-necessarily free R-modules
------------------------------------------+---------------------------------
Reporter: was | Owner: was
Type: enhancement | Status: new
Priority: major | Milestone: sage-4.1
Component: linear algebra | Keywords:
Reviewer: Robert Miller, John Palmieri | Author: William Stein
Merged: |
------------------------------------------+---------------------------------
Changes (by jhpalmieri):
* reviewer: Robert Miller => Robert Miller, John Palmieri
Comment:
A partial review; I've looked at the files *not* in the fg_pid directory,
and I'm happy with them except for the minor comments below about
free_module_morphism.py. I'm now working through the files in the fg_pid
directory, and I'm about halfway through fgp_module.py.
*******************************************
In free_module_morphism.py:
in {{{inverse_image}}}, is there any speed gain if you check whether the
rank of the homomorphism is zero at the beginning? that is, change
{{{
if self.domain().rank() == 0:
return self.domain()
}}}
to
{{{
if self.domain().rank() == 0 or self.rank() == 0:
return self.domain()
}}}
or even to
{{{
if self.rank() == 0:
return self.domain()
}}}
A very minor issue: In {{{inverse_image}}}, in the lines
{{{
if R.is_field():
# By solving, find lifts of each of the basis elements of V.
# Each row of Y gives a linear combination of the basis for the
domain
# that maps to one of the basis elements V.
C = A.solve_left(B)
}}}
change "Each row of Y" to "Each row of C"?
*******************************************
fgp_module.py:
the function FGP_Module: documentation should say what it returns: V/W as
a f.g. R-module. For the inputs, V and W need to be free R-modules.
_coerce_map_from_: I think that V/W should have a coercion from V, so
testing isinstance(S, FGP_Module_class) won't work. More generally, you
should have coercions from anything of the form V/W' to V/W, as long as W'
is contained in W: that is, you should have coercions not just from
submodules, but also from modules with canonical surjections onto V/W.
(There may be many maps from V to V/W, but there is one *canonical* map,
so I think there should be a coercion map.)
in free_module.py, there is a {{{__cmp__}}} method but not an {{{__eq__}}}
method, and here it's the other way around. Is this an issue? Do we have
a policy about which one to use?
Here's a problem with is_submodule:
{{{
sage: V = FreeModule(ZZ, 1)
sage: W = V.submodule([0*V.0])
sage: W2 = V.submodule([2*V.0])
sage: V/W # isomorphic to Z
Finitely generated module V/W over Integer Ring with invariants (0)
sage: V/W2 # isomorphic to Z/2Z
Finitely generated module V/W over Integer Ring with invariants (2)
sage: (V/W).is_submodule(V/W2)
True
}}}
This is because the implementation is wrong: if you want V/W to be a
submodule of V'/W', then you want V to be a submodule of V', and the
inclusion map should induce a surjection of W onto W' (hence an
isomorphism between W and W' -- given an injection V --> V' which induces
a map W --> W', the induced map must be an injection also). Should you
change the end from
{{{
return self.V().is_submodule(A.V()) and
self.W().is_submodule(A.W())
}}}
to
{{{
return self.V().is_submodule(A.V()) and self.W() == A.W()
}}}
The docstring should be changed to reflect this, as should the doctests;
in the existing doctests, I don't think that A is actually a submodule of
Q. In the simpler (but somewhat analogous) situation of Z/2Z and Z/4Z,
then although abstractly Z/2Z is a submodule of Z/4Z, the identity map Z
--> Z does not induce the inclusion. The identity map Z --> Z induces an
inclusion 4Z --> 2Z, which then induces a *surjection* Z/4Z --> Z/2Z.
Also, the first sentence of the docstring is wrong: it says "Return True
if A is a submodule of self", and it should be "Return True if self is a
submodule of A".
base_ring: delete the comment ({{{# this will be done in __init__}}})?
{{{_smith_form_}}}: document the form of the output, or at least give a
reference to smith_form from matrix2.pyx.
invariants needs a docstring, not just doctests. (The term "invariants"
is used frequently in fgp_module.py, but is never defined.) Document the
argument "include_ones"?
gens: in the docstring, change "lements" to "elements". Change it to
"elements g0,...,gn of self"? (Add "of self"?)
is it worth changing "isinstance(x, FGP_Module_class)" to
"is_FGP_Module(x)" many places in this file? (Otherwise, is
"is_FGP_Module" used anywhere? We could delete it...)
I'm still working on this file, and I haven't looked at fgp_morphism or
fgp_element in any detail yet (except to note that the in the docstring
for FGP_Morphism, the comment "where the morphism doesn't extend to V1"
needs to be rephrased). I'm going on a family trip tomorrow, though, so I
may not have time to work on this for a few days.
--
Ticket URL: <http://trac.sagemath.org/sage_trac/ticket/5882#comment:29>
Sage <http://sagemath.org/>
Sage: Creating a Viable Open Source Alternative to Magma, Maple, Mathematica,
and MATLAB
--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"sage-trac" group.
To post to this group, send email to [email protected]
To unsubscribe from this group, send email to
[email protected]
For more options, visit this group at
http://groups.google.com/group/sage-trac?hl=en
-~----------~----~----~----~------~----~------~--~---
