#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 tscrim):
Replying to [comment:65 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?
That must of gotten lost when I had connectivity issues. This was suppose
to be "by using a consistent API".
> > > - 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.
I thank them both for working on this and pushing it forward.
> 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.
I don't want to diminish their work, but sometimes after having the code
there and looking ahead, major refactoring can be needed. However, there
would likely need to be refactoring of the usual commutative polynomials
to address the issue you brought up. There could also be ways to factor
out common code from other algebras across Sage. So in short, what I'm
suggesting is to have a common abstract base class for both for the
basic/core functionality. If there is only a little overlap (I really
haven't looked), then we can just continue in our current fashion.
--
Ticket URL: <https://trac.sagemath.org/ticket/13215#comment:67>
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.
