#13215: Skew polynomials
-------------------------------------+-------------------------------------
Reporter: caruso | Owner: tbd
Type: enhancement | Status: needs_work
Priority: major | Milestone: sage-7.3
Component: algebra | Resolution:
Keywords: skew polynomials | Merged in:
Authors: Xavier Caruso | Reviewers: Burcin Erocal
Report Upstream: N/A | Work issues:
Branch: | Commit:
u/arpitdm/skew_polynomials | e189fec13d005a7fba39a429876c501fe95c05da
Dependencies: #13214, #13303, | Stopgaps:
#13640, #13641, #13642 |
-------------------------------------+-------------------------------------
Comment (by jsrn):
Replying to [comment:60 tscrim]:
> It is okay to change class names as long as the public API doesn't
change (although this can entail unpickling issues in either case). I
would instead future-proof this by
>
Was there supposed to be some ending of this sentence?
> > - There's an option `sparse=False` which, if set to `True`, returns a
`NotImplementedError`.
> > I suggest to remove it for now in order to not confuse the user, and
reinstate it later
> > when it will be fully implemented. In this case, also remove all
references to this feature
> > in the documentation.
> > - As I suggest to remove some not implemented but planned features, it
might be a good
> > idea to add a `TODO` block as a reminder of these features (sparse
rings, multivariate rings).
>
> I disagree with these comments. This is documenting an upcoming feature,
it helps keeps the API consistent with future plans and other parts of
Sage, and tells someone reading the code what is currently not
implemented.
This is debatable: sparse skew polynomials are not very useful whenever
there is a derivation (since sparsity is not retained well across
operations), and it's not very clear what a multivariate skew ring is. The
most natural generalisation of this structure is Ore polynomials, but that
would likely be a separate class. I don't think it makes sense to leave
implementation artifacts that will, most likely, never lead to real
implementations. In the odd case that they *would* lead to
implementations, we could reinstate them with no penalty.
> Also, from a cursory glance, there seems to be a lot of duplication with
the current implementations of polynomials (i.e., the elements). I feel
there could be a lot of simplification of the code by either subclassing
or creating a mix-in (cython) class that overrides the `_mul_` of
polynomials. Is there a reason why you didn't do this? This might entail
doing some refactoring/abstracting of the polynomial code though, but it
should make the implementation of skew polynomials a lot easier (and bug
proof).
That's arguably true. To be fair: Xavier Caruso made the code, and Arpit
just tried to get it into shape with less than full-rewrite-workload. That
said, I would be concerned about subclassing commutative classes that
commutative-only code creeps into the skew polynomials: for instance if
someone later on adds a method to the parent class and forgets to
explicitly overwrite or remove this method in the skew polynomial class.
It's really only all the basics that are shared btw commutative/non-
commutative: all the substantial and complex properties are not shared at
all.
--
Ticket URL: <https://trac.sagemath.org/ticket/13215#comment:65>
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 https://groups.google.com/group/sage-trac.
For more options, visit https://groups.google.com/d/optout.
