Convenience syntax for quaternion ideals

[yyyyx4]

Currently (sage 9.3), quaternion ideals do not support some of the operations one may expect them to:

• Trying to scale an ideal I on the left by an element a by writing a*I throws a TypeError.
• Trying to scale an ideal I on the right by an element a by writing I*a throws an AttributeError (this seems to be because the _scale method was renamed to scale at some point).
• Trying to compute the sum of two ideals I and J by writing I+J "works", but it returns a generic Twosided Ideal (which, by the way, is usually incorrect) rather than a quaternion ideal.
• Trying to construct a one-sided principal ideal of a quaternion order O by writing a*O or O*a throws a NotImplementedError.

This patch makes all of these things work. There's a good chance I messed up something with respect to sage's coercion system though, so I am hoping for someone to educate me on this matter.

As a side note, given the issue with the implementation of + inherited from generic ideals, I'm not sure we should be deriving quaternion ideals from the generic ideal class in the first place.

Depends on #32245

Component: algebra

Keywords: quaternion ideals

Author: Lorenz Panny

Branch/Commit: eef228d

Reviewer: Peter Bruin

Issue created by migration from https://trac.sagemath.org/ticket/32264

[pjbruin]

comment:2

After #31582 is merged, the way to make addition work for ideals is probably to implement the single-underscore method _add_ instead of __add__ (not yet for orders, probably), because quaternion ideals will then have a proper parent. Also, it is possible that the the fact that the _richcmp_() method implemented in #31582 will take care of the FIXME.

[pjbruin]

comment:3

I now see that you also implemented a parent for quaternion ideals. I hope that this can somehow be combined with the changes made in #31582, but it seems to be a little bit less straightforward than I thought.

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. New commits:

3ef029d	Merge branch 'ticket/31582-quaternion_ideal_parent' into ticket/32245-quaternion_ideal_basis
2835449	Merge branch 'public/fix_quaternion_scaled_ideals_equality' into public/more_convenient_quaternion_ideals

[sagetrac-git]

Changed commit from 12e6398 to 2835449

[yyyyx4]

comment:5

I think I successfully rebased this on top of your changes.

By the way, I couldn't get _add_ to work because the implementation of __add__ in Ideal_generic does not call the single-underscore method. Presumably this can be changed, but I didn't dare touching that code.

[pjbruin]

comment:6

Looks good. Multiplication doesn't seem to work yet when putting an order on the right, though:

sage: Q = QuaternionAlgebra(3, 5)
sage: O = Q.maximal_order()
sage: O * O.unit_ideal()
Fractional ideal (1/2 + 1/2*k, 1/2*i + 1/2*j, j, k)
sage: O.unit_ideal() * O
Traceback (most recent call last):
...
NotImplementedError: object does not support iteration
sage: O * O
Traceback (most recent call last):
...
NotImplementedError: object does not support iteration

Also, do you know if the two lines __radd__ = __add__ are necessary? The __radd__() method seems to be used very little in Sage.

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. New commits:

eef228d

fix ideal*order multiplications; remove unneeded `__radd__` methods

[sagetrac-git]

Changed commit from 2835449 to eef228d

[yyyyx4]

comment:8

Thanks, good catch. Both fixed.

(I think I first defined one of the __radd__s when the QuaternionOrder didn't yet have its own __add__ so it could be handled by the ideal on the left instead, but now that's indeed obsolete.)

[pjbruin]

Reviewer: Peter Bruin

[pjbruin]

comment:9

Thanks! Positive review modulo #32245, which I guess someone else will have to review.

[slel]

comment:11

Setting milestone to 9.6 now that 9.5 is out.

[vbraun]

Changed branch from public/more_convenient_quaternion_ideals to eef228d