#29678 closed enhancement (fixed)

Fraction field of Ore polynomial ring

Reported by: caruso Owned by:
Priority: major Milestone: sage-9.2
Component: algebra Keywords:
Cc: Merged in:
Authors: Xavier Caruso Reviewers: Travis Scrimshaw
Report Upstream: N/A Work issues:
Branch: c40eebe (Commits, GitHub, GitLab) Commit: c40eebead49dba2a9b37a3c1897a7eb71688296e
Dependencies: #29629 Stopgaps:

Status badges

Description

We implement the fraction field of Ore polynomial rings.

The elements of the fraction field are represented by fractions of the form denom^(-1) * num.

Change History (41)

comment:1 Changed 17 months ago by caruso

  • Branch set to u/caruso/ore_functions

comment:2 Changed 17 months ago by caruso

  • Authors set to Xavier Caruso
  • Commit set to 2e9458baa421e1aba7d56fa3fe9eb8a5ff2d411e
  • Dependencies changed from #29269 to #29629

Last 10 new commits:

52932e0fix typos
31a3c07add documentation
0a232cdLaTeX representation of derivations
d456551implement Hilbert shift
f225cbcfix pyflakes
9067b73Ore functions (first draft)
d0a9712Hilbert shift
650079dpolish implementation
f6835a3implement extend_to_fraction_field for derivations
2e9458bregister coercion from a Ore ring to its fraction field

comment:3 Changed 17 months ago by git

  • Commit changed from 2e9458baa421e1aba7d56fa3fe9eb8a5ff2d411e to a74ddac387d89650325b92ef53237343fb7b2ac9

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

a74ddacreduced trace and reduced norm

comment:4 Changed 17 months ago by git

  • Commit changed from a74ddac387d89650325b92ef53237343fb7b2ac9 to f50aca6f0d42e964521c5b327848ec33f8896c36

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

7cf9e02doctest in ore_function_field.py
f50aca6more doctests

comment:5 Changed 16 months ago by mkoeppe

  • Milestone changed from sage-9.1 to sage-9.2

comment:6 Changed 15 months ago by git

  • Commit changed from f50aca6f0d42e964521c5b327848ec33f8896c36 to 648cf6d95830c5ae4279415019ce73ef4bd6758d

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

0c8f04dMerge branch 'develop' into t/29678/ore_functions
b35e1d0fix one doctest
131d78eMerge branch 'develop' into t/29629/ore_polynomials_rebased
4f273e8Merge branch 't/29629/ore_polynomials_rebased' into t/29678/ore_functions
648cf6dremove trailing spaces

comment:7 Changed 15 months ago by git

  • Commit changed from 648cf6d95830c5ae4279415019ce73ef4bd6758d to 15edb980ec75201977aebfd74afdee1a04497ba7

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

15edb98fix pickling

comment:8 Changed 15 months ago by caruso

  • Status changed from new to needs_review

There is still a small issue with category (elements are not instances of parent.category().element_class) but I don't know how I should fix it. Any ideas?

Last edited 15 months ago by caruso (previous) (diff)

comment:9 Changed 15 months ago by tscrim

Typically that means you are returning elements that are not instances of ParentObject.element_class. If this is for a morphism parent, you can simply silence that warning. (This is a common practice as there can be many morphism types, but we don't (yet) have a good framework for these things. I have seen a workaround for this that just dynamically creates the new element class, but I forget where in the Sage code this is done.)

comment:10 Changed 15 months ago by git

  • Commit changed from 15edb980ec75201977aebfd74afdee1a04497ba7 to 070792b9c20e7e9f0c7f2aaac0989dd8b6c53c7f

Branch pushed to git repo; I updated commit sha1. Last 10 new commits:

445913aabstract method degree raises NotImplementedError
fdbded7add full-stop after math formulas
c778356remove import sage
bf17a36Merge branch 'develop' of https://github.com/sagemath/sage into t/29629/ore_polynomials_rebased
f7b0eddMerge branch 't/29629/ore_polynomials_rebased' into t/29678/ore_functions
94a1a5clazy import
15f6dfafix indentation
a9bae69update documentation
3cf021aMerge branch 't/29629/ore_polynomials_rebased' into t/29678/ore_functions
070792blet extend_to_fraction_field be more tolerant

comment:11 Changed 15 months ago by caruso

Here is my issue:

sage: k.<a> = GF(5^3)
sage: Frob = k.frobenius_endomorphism()
sage: S.<x> = k['x', Frob]
sage: K = S.fraction_field()
sage: K(x)._test_category()
Traceback (most recent call last):
...
AssertionError: False is not true

K.category().element_class is sage.categories.algebras.Algebras.element_class but K(x) is an instance is an instance of sage.rings.polynomial.ore_function_element.OreFunction_with_large_center which, apparently, does not define from the former (although OreFunction_with_large_center derives from AlgebraElement).

Last edited 15 months ago by caruso (previous) (diff)

comment:12 follow-up: Changed 15 months ago by tscrim

This should be your problem: In OreFunctionField._element_constructor_, you need to change

-self.Element(self, *args, **kwds)
+self.element_class(self, *args, **kwds)

This also needs to be changed for OrePolynomialRing._element_constructor_:

-C = self.Element
+C = self.element_class

One other thing: the extend_to_fraction_field needs doctests.

comment:13 Changed 15 months ago by git

  • Commit changed from 070792b9c20e7e9f0c7f2aaac0989dd8b6c53c7f to 8c0efe704d0dee6efb6267b45150fd0bd7d6cea0

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

c7d0acdreorganize documentation
ec0e24bElement -> element_class
8c0efe7add a doctest

comment:14 in reply to: ↑ 12 ; follow-up: Changed 15 months ago by caruso

Replying to tscrim:

This should be your problem: In OreFunctionField._element_constructor_, you need to change

-self.Element(self, *args, **kwds)
+self.element_class(self, *args, **kwds)

This also needs to be changed for OrePolynomialRing._element_constructor_:

-C = self.Element
+C = self.element_class

Thanks, It indeed fixes the problem!

One other thing: the extend_to_fraction_field needs doctests.

There were already doctests. But I've added one more.

comment:15 Changed 15 months ago by git

  • Commit changed from 8c0efe704d0dee6efb6267b45150fd0bd7d6cea0 to 44c9913ab197d0c13f7d24a055c987d0257c5f31

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

44c9913typos

comment:16 in reply to: ↑ 14 Changed 15 months ago by tscrim

Replying to caruso:

Replying to tscrim:

One other thing: the extend_to_fraction_field needs doctests.

There were already doctests. But I've added one more.

Wrong method: RingDerivationWithoutTwist.extend_to_fraction_field. Sorry, I didn't realize there was more than one.

comment:17 Changed 15 months ago by tscrim

Sorry, two places: RingDerivationWithTwist_generic.extend_to_fraction_field as well.

comment:18 Changed 15 months ago by git

  • Commit changed from 44c9913ab197d0c13f7d24a055c987d0257c5f31 to e1fc6605e40127638cbe28bd130e7c8abb73cbf7

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

e1fc660more doctests

comment:19 Changed 15 months ago by caruso

Fixed.

Also, I tried to reorganize a bit the documentation and separate commutative polynomials from noncommutative polynomials. I created a new file doc/en/reference/noncommutative_polynomial_rings/index.rst but I probably did something wrong because this file is not found when building the documentation. Do you know what's going on, here?

comment:20 follow-up: Changed 15 months ago by tscrim

I think you also need the conf.py file.

comment:21 Changed 15 months ago by git

  • Commit changed from e1fc6605e40127638cbe28bd130e7c8abb73cbf7 to f6ea4fdfb95860f21606bf80fad91d1bc1600a43

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

1cc0ed8Merge branch 'develop' into t/29629/ore_polynomials_rebased
59d3ed2Merge branch 't/29629/ore_polynomials_rebased' into t/29678/ore_functions
200d7bdupdate doctests
f6ea4fdMerge branch 't/29629/ore_polynomials_rebased' into t/29678/ore_functions

comment:22 Changed 15 months ago by git

  • Commit changed from f6ea4fdfb95860f21606bf80fad91d1bc1600a43 to 548a1dcdd1e70a703a7a2ec06f5f326519e7a92b

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

548a1dcadd conf.py

comment:23 in reply to: ↑ 20 Changed 15 months ago by caruso

Replying to tscrim:

I think you also need the conf.py file.

Oh, I see. Thanks.

comment:24 Changed 15 months ago by tscrim

Rather than be a direct file that refers back to conf_sub.py, the other folders are symlinks. Could you do the same thing here?

comment:25 Changed 15 months ago by caruso

My conf.py is a symlink, isn't it?

comment:26 Changed 15 months ago by git

  • Commit changed from 548a1dcdd1e70a703a7a2ec06f5f326519e7a92b to d3c744ee08178f1b541cc2c6b865b83a106f72cd

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

d3c744eaccept more errors in trying to inverse the twisting morphism

comment:27 Changed 15 months ago by git

  • Commit changed from d3c744ee08178f1b541cc2c6b865b83a106f72cd to 7e3abda2c626de0a3370d4f8bec2c00f39cb490c

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

7e3abdatypos in doctests

comment:28 Changed 15 months ago by tscrim

Ah, I see. Git is just handling it in a strange way. Sorry for the noise.

However, no bare excepts.

comment:29 Changed 15 months ago by caruso

Btw, I saw that there are pyflakes warnings in conf.py due to the imports (on two consecutive lines):

from sage.docs.conf import release, exclude_patterns
from sage.docs.conf import *

Shouldn't we remove the second import?

Last edited 15 months ago by caruso (previous) (diff)

comment:30 follow-up: Changed 15 months ago by tscrim

I wouldn't touch the conf.py files here. That should be a separate ticket as that is outside of the scope and could have larger impact to Sage's doc.

comment:31 in reply to: ↑ 30 Changed 15 months ago by caruso

Replying to tscrim:

I wouldn't touch the conf.py files here. That should be a separate ticket as that is outside of the scope and could have larger impact to Sage's doc.

Yes, I agree.

comment:32 Changed 15 months ago by tscrim

The only thing missing from me giving this a positive review is taking care of the bare except: (this can catch keyboard interrupts I've been told, so it is essentially prohibited to have in Sage code).

comment:33 Changed 15 months ago by tscrim

  • Branch changed from u/caruso/ore_functions to u/tscrim/ore_functions-29678
  • Commit changed from 7e3abda2c626de0a3370d4f8bec2c00f39cb490c to 1c7a67c8638b3984256281b5da382daf69ec36ae

Here is a version with my docbuild fixes from #29629.


New commits:

f111d30Merge branch 'u/caruso/ore_polynomials_rebased' of git://trac.sagemath.org/sage into u/caruso/ore_polynomials_rebased
5d822f3Fixing the doc and != for polynomial injection maps.
1c7a67cMerge branch 'u/tscrim/ore_polynomials-29629' of git://trac.sagemath.org/sage into u/caruso/ore_polynomials-29629

comment:34 Changed 15 months ago by caruso

  • Branch changed from u/tscrim/ore_functions-29678 to u/caruso/ore_functions-29678

comment:35 Changed 15 months ago by caruso

  • Commit changed from 1c7a67c8638b3984256281b5da382daf69ec36ae to b327f519bd14c3086372bbf8dbd4c89b60b191ea

OK, I've added an explicit list of exceptions.

I also rebased the commit on my branch because I thought you have forgotten to include the core of this ticket in your merge :-). I hope everything is now merged correctly.


Last 10 new commits:

8c0efe7add a doctest
44c9913typos
e1fc660more doctests
59d3ed2Merge branch 't/29629/ore_polynomials_rebased' into t/29678/ore_functions
f6ea4fdMerge branch 't/29629/ore_polynomials_rebased' into t/29678/ore_functions
548a1dcadd conf.py
d3c744eaccept more errors in trying to inverse the twisting morphism
7e3abdatypos in doctests
055bdebMerge branch 't/29629/ore_polynomials_rebased' into t/29678/ore_functions
b327f51add list of exceptions

comment:36 Changed 15 months ago by tscrim

Whoops, bad copy-paste of the branch name. ^^;; If the patchbot comes back green, then positive review. Thank you.

comment:37 Changed 15 months ago by caruso

The patchbots reported a bunch of errors in the documentation (some files are not found). However, I'm not sure that they are related to this ticket. Do you know what's happening here?

comment:38 Changed 15 months ago by tscrim

In ore_polynomial_ring.py:

-:module:`sage.rings.polynomial.ore_function_field`
+:mod:`sage.rings.polynomial.ore_function_field`

That is at least the current docbuild failure.

comment:39 Changed 15 months ago by git

  • Commit changed from b327f519bd14c3086372bbf8dbd4c89b60b191ea to c40eebead49dba2a9b37a3c1897a7eb71688296e

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

c40eebe:module: -> :mod:

comment:40 Changed 15 months ago by tscrim

  • Reviewers set to Travis Scrimshaw
  • Status changed from needs_review to positive_review

Thank you. LGTM.

comment:41 Changed 14 months ago by vbraun

  • Branch changed from u/caruso/ore_functions-29678 to c40eebead49dba2a9b37a3c1897a7eb71688296e
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.