#18600: Fix several methods for sparse polynomials
-------------------------------------+-------------------------------------
Reporter: bruno | Owner:
Type: defect | Status: needs_review
Priority: major | Milestone: sage-6.9
Component: commutative | Resolution:
algebra | Merged in:
Keywords: sparse polynomial | Reviewers: Vincent Delecroix
Authors: Bruno Grenet | Work issues:
Report Upstream: N/A | Commit:
Branch: | 809bc3ea6da046dd4bf7582c44ea8ef90319ea73
u/bruno/polynomial_sparse | Stopgaps:
Dependencies: |
-------------------------------------+-------------------------------------
Changes (by bruno):
* status: needs_work => needs_review
Comment:
Replying to [comment:3 vdelecroix]:
> * `integral`
>
> What about {{{Q = self.base_ring().fraction_field()}}} instead of
{{{Q = (self.constant_coefficient()/1).parent()}}}?
Right, I do not remember why I wrote it in such a weird way... Corrected!
>
> The `.change_ring` method is very slow but this has nothing to do
with this ticket.
And actually, a large part is already taken to simply build the new
polynomial ring. For instance, on my laptop, `change_ring` takes
approximately 12µs, of which 9µs are taken for building the new
`PolynomialRing`. Yet the extra 3µs shouldn't exist...
>
> * `is_unit`
>
> Why do you care that the base ring being an integral domain? Is `1`
not a unit in `Z/4Z`? Sage thinks it is
> {{{
> sage: Zmod(4)(1).is_unit()
> True
> }}}
You may have overlooked the code: If the base ring is an integral domain
and the input polynomial has degree > 0, it cannot be a unit so I directly
return `False`. I test the nilpotency of the coefficients only if the base
ring is not an integral domain.
> * `reverse`
>
> You can use `for k,v in self.__coeffs.iteritems()`
>
> * `integral` for sparse
>
> idem: use `iteritems`. In other words, replace
> {{{
> Q({k:self.__coeffs[k].integral(var) for k in self.__coeffs.keys()})
> }}}
> with
> {{{
> Q({k:v.integral(var) for k,v in self.__coeffs.iteritems()})
> }}}
> I found strange that in one case you defined the dictionary `d` and
then `return Q(d)` while in the other you do everything at once.
Comments taken into account.
--
Ticket URL: <http://trac.sagemath.org/ticket/18600#comment:5>
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.
