Opened 2 years ago
Closed 2 years ago
#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: |
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 2 years ago by
- Branch set to u/caruso/ore_functions
comment:2 Changed 2 years ago by
- Commit set to 2e9458baa421e1aba7d56fa3fe9eb8a5ff2d411e
- Dependencies changed from #29269 to #29629
comment:3 Changed 2 years ago by
- Commit changed from 2e9458baa421e1aba7d56fa3fe9eb8a5ff2d411e to a74ddac387d89650325b92ef53237343fb7b2ac9
Branch pushed to git repo; I updated commit sha1. New commits:
a74ddac | reduced trace and reduced norm
|
comment:4 Changed 2 years ago by
- Commit changed from a74ddac387d89650325b92ef53237343fb7b2ac9 to f50aca6f0d42e964521c5b327848ec33f8896c36
comment:5 Changed 2 years ago by
- Milestone changed from sage-9.1 to sage-9.2
comment:6 Changed 2 years ago by
- Commit changed from f50aca6f0d42e964521c5b327848ec33f8896c36 to 648cf6d95830c5ae4279415019ce73ef4bd6758d
Branch pushed to git repo; I updated commit sha1. New commits:
0c8f04d | Merge branch 'develop' into t/29678/ore_functions
|
b35e1d0 | fix one doctest
|
131d78e | Merge branch 'develop' into t/29629/ore_polynomials_rebased
|
4f273e8 | Merge branch 't/29629/ore_polynomials_rebased' into t/29678/ore_functions
|
648cf6d | remove trailing spaces
|
comment:7 Changed 2 years ago by
- Commit changed from 648cf6d95830c5ae4279415019ce73ef4bd6758d to 15edb980ec75201977aebfd74afdee1a04497ba7
Branch pushed to git repo; I updated commit sha1. New commits:
15edb98 | fix pickling
|
comment:8 Changed 2 years ago by
- 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?
comment:9 Changed 2 years ago by
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 2 years ago by
- Commit changed from 15edb980ec75201977aebfd74afdee1a04497ba7 to 070792b9c20e7e9f0c7f2aaac0989dd8b6c53c7f
Branch pushed to git repo; I updated commit sha1. Last 10 new commits:
445913a | abstract method degree raises NotImplementedError
|
fdbded7 | add full-stop after math formulas
|
c778356 | remove import sage
|
bf17a36 | Merge branch 'develop' of https://github.com/sagemath/sage into t/29629/ore_polynomials_rebased
|
f7b0edd | Merge branch 't/29629/ore_polynomials_rebased' into t/29678/ore_functions
|
94a1a5c | lazy import
|
15f6dfa | fix indentation
|
a9bae69 | update documentation
|
3cf021a | Merge branch 't/29629/ore_polynomials_rebased' into t/29678/ore_functions
|
070792b | let extend_to_fraction_field be more tolerant
|
comment:11 Changed 2 years ago by
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
).
comment:12 follow-up: ↓ 14 Changed 2 years ago by
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 2 years ago by
- Commit changed from 070792b9c20e7e9f0c7f2aaac0989dd8b6c53c7f to 8c0efe704d0dee6efb6267b45150fd0bd7d6cea0
comment:14 in reply to: ↑ 12 ; follow-up: ↓ 16 Changed 2 years ago by
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 2 years ago by
- Commit changed from 8c0efe704d0dee6efb6267b45150fd0bd7d6cea0 to 44c9913ab197d0c13f7d24a055c987d0257c5f31
Branch pushed to git repo; I updated commit sha1. New commits:
44c9913 | typos
|
comment:16 in reply to: ↑ 14 Changed 2 years ago by
comment:17 Changed 2 years ago by
Sorry, two places: RingDerivationWithTwist_generic.extend_to_fraction_field
as well.
comment:18 Changed 2 years ago by
- Commit changed from 44c9913ab197d0c13f7d24a055c987d0257c5f31 to e1fc6605e40127638cbe28bd130e7c8abb73cbf7
Branch pushed to git repo; I updated commit sha1. New commits:
e1fc660 | more doctests
|
comment:19 Changed 2 years ago by
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: ↓ 23 Changed 2 years ago by
I think you also need the conf.py
file.
comment:21 Changed 2 years ago by
- Commit changed from e1fc6605e40127638cbe28bd130e7c8abb73cbf7 to f6ea4fdfb95860f21606bf80fad91d1bc1600a43
Branch pushed to git repo; I updated commit sha1. New commits:
1cc0ed8 | Merge branch 'develop' into t/29629/ore_polynomials_rebased
|
59d3ed2 | Merge branch 't/29629/ore_polynomials_rebased' into t/29678/ore_functions
|
200d7bd | update doctests
|
f6ea4fd | Merge branch 't/29629/ore_polynomials_rebased' into t/29678/ore_functions
|
comment:22 Changed 2 years ago by
- Commit changed from f6ea4fdfb95860f21606bf80fad91d1bc1600a43 to 548a1dcdd1e70a703a7a2ec06f5f326519e7a92b
Branch pushed to git repo; I updated commit sha1. New commits:
548a1dc | add conf.py
|
comment:23 in reply to: ↑ 20 Changed 2 years ago by
comment:24 Changed 2 years ago by
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 2 years ago by
My conf.py
is a symlink, isn't it?
comment:26 Changed 2 years ago by
- Commit changed from 548a1dcdd1e70a703a7a2ec06f5f326519e7a92b to d3c744ee08178f1b541cc2c6b865b83a106f72cd
Branch pushed to git repo; I updated commit sha1. New commits:
d3c744e | accept more errors in trying to inverse the twisting morphism
|
comment:27 Changed 2 years ago by
- Commit changed from d3c744ee08178f1b541cc2c6b865b83a106f72cd to 7e3abda2c626de0a3370d4f8bec2c00f39cb490c
Branch pushed to git repo; I updated commit sha1. New commits:
7e3abda | typos in doctests
|
comment:28 Changed 2 years ago by
Ah, I see. Git is just handling it in a strange way. Sorry for the noise.
However, no bare excepts.
comment:29 Changed 2 years ago by
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?
comment:30 follow-up: ↓ 31 Changed 2 years ago by
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 2 years ago by
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 2 years ago by
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 2 years ago by
- 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:
f111d30 | Merge branch 'u/caruso/ore_polynomials_rebased' of git://trac.sagemath.org/sage into u/caruso/ore_polynomials_rebased
|
5d822f3 | Fixing the doc and != for polynomial injection maps.
|
1c7a67c | Merge branch 'u/tscrim/ore_polynomials-29629' of git://trac.sagemath.org/sage into u/caruso/ore_polynomials-29629
|
comment:34 Changed 2 years ago by
- Branch changed from u/tscrim/ore_functions-29678 to u/caruso/ore_functions-29678
comment:35 Changed 2 years ago by
- 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:
8c0efe7 | add a doctest
|
44c9913 | typos
|
e1fc660 | more doctests
|
59d3ed2 | Merge branch 't/29629/ore_polynomials_rebased' into t/29678/ore_functions
|
f6ea4fd | Merge branch 't/29629/ore_polynomials_rebased' into t/29678/ore_functions
|
548a1dc | add conf.py
|
d3c744e | accept more errors in trying to inverse the twisting morphism
|
7e3abda | typos in doctests
|
055bdeb | Merge branch 't/29629/ore_polynomials_rebased' into t/29678/ore_functions
|
b327f51 | add list of exceptions
|
comment:36 Changed 2 years ago by
Whoops, bad copy-paste of the branch name. ^^;;
If the patchbot comes back green, then positive review. Thank you.
comment:37 Changed 2 years ago by
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 2 years ago by
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 2 years ago by
- Commit changed from b327f519bd14c3086372bbf8dbd4c89b60b191ea to c40eebead49dba2a9b37a3c1897a7eb71688296e
Branch pushed to git repo; I updated commit sha1. New commits:
c40eebe | :module: -> :mod:
|
comment:40 Changed 2 years ago by
- Reviewers set to Travis Scrimshaw
- Status changed from needs_review to positive_review
Thank you. LGTM.
comment:41 Changed 2 years ago by
- Branch changed from u/caruso/ore_functions-29678 to c40eebead49dba2a9b37a3c1897a7eb71688296e
- Resolution set to fixed
- Status changed from positive_review to closed
Last 10 new commits:
fix typos
add documentation
LaTeX representation of derivations
implement Hilbert shift
fix pyflakes
Ore functions (first draft)
Hilbert shift
polish implementation
implement extend_to_fraction_field for derivations
register coercion from a Ore ring to its fraction field