#18066: Cleanup of ModulesWithBasis and friends
-------------------------------------+-------------------------------------
Reporter: tscrim | Owner: tscrim
Type: enhancement | Status: needs_review
Priority: major | Milestone: sage-6.10
Component: categories | Resolution:
Keywords: | Merged in:
Authors: Travis Scrimshaw | Reviewers:
Report Upstream: N/A | Work issues:
Branch: | Commit:
public/categories/cleanup_CFM_modules_wBasis-18066|
954e3542ccc00ef3cc294f19019da3608d87a5a8
Dependencies: | Stopgaps:
-------------------------------------+-------------------------------------
Changes (by tscrim):
* cc: aschilling, zabrocki, jhpalmieri (added)
Comment:
So in my latest commit I fixed doctests. Some were trivial due to
`support` no longer being ordered (and hence changes to the k-schur book
tests and the cc of Anne and Mike), some were more substantial:
- `ChainComplexes` when passed a field said `VectorSpaces`, and so it did
not consider itself as having a distinguished basis. However, if you give
it a ring (like `ZZ`), then it became in `ModulesWithBasis`
(=`FreeModules`), which it currently does not live from what I saw. Since
there wasn't anything using this extra functionality (any attempt to use
it was likely broken), I opted to downgrade the category to just
`Modules`. (John, I cc'ed you to let you know.)
- I added a `dense_coefficient_list` for finite dimensional modules with
basis so that you could get an ordered list of coefficients including 0's.
This was needed for hyperplane arrangements. I'm open to different names.
- I made the linear expression code be in finite dimensional modules with
basis.
Replying to [comment:3 darij]:
> However, one thing I'm still missing is an explicit explanation of what
methods one needs to implement in order to inherit from
`ModulesWithBasis`. I suspect this is no longer up-to-date:
> {{{
> # To implement a module_with_basis you need to implement either
> # basis() or an _indices attribute and monomial().
> }}}
> (as in, we probably need more now), right?
I put that in there because that is all that should be required. However,
how the code breaks and the traceback tells you everything you need to
know as far as what to implement.
> Coercion of coefficients into the base ring needs to be carefully
thought over. As of now, I am not sure if it works correctly. For
instance, `term` does no coercion, but `__invert__` does
`self.parent().term(one, ~mcs[one])`, although the inversion operator `~`
might produce an element of a bigger ring (for example, `~(ZZ(2))` returns
`1/2`).
Actually my generic implementation should handle this with grace as it is
`coeff * self.monomial(index)`. However, CFM currently does not do any
coercion as indicated in the docstring, but that is a discussion for
another ticket.
> One more thing, which is probably not your fault but just caught my
eyes. The docstring of `module_morphism` says:
> {{{
> - ``matrix`` -- a matrix of size `\dim X \times \dim Y`
> or `\dim Y \times \dim X`
> }}}
> I don't see why the "or" is here. The doctests show that `\dim Y \times
\dim X` works, but does `\dim X \times \dim Y` work too? And if so,
*should* it? I think checking whether the matrix has the "wrong"
dimensions, and then trying to fix them by transposing it, would be a
brittle paradigm.
I don't know and I don't really care as that is outside of the scope of
this ticket.
> Also, could we have doctests proving that the `monomial_coefficients`
method works correctly (including not mutating the dictionary unless
explicitly required) on various instances of `ModulesWithBasis` (e.g.,
vector spaces)?
Added.
--
Ticket URL: <http://trac.sagemath.org/ticket/18066#comment:6>
Sage <http://www.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 unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To post to this group, send email to [email protected].
Visit this group at http://groups.google.com/group/sage-trac.
For more options, visit https://groups.google.com/d/optout.
