Opened 6 years ago
Closed 5 years ago
#23190 closed enhancement (fixed)
Improve is_injective()/is_surjective() for coercions of quotient rings
Reported by:  Julian Rüth  Owned by:  

Priority:  minor  Milestone:  sage8.1 
Component:  commutative algebra  Keywords:  sd86.5, sd87, beginner 
Cc:  Merged in:  
Authors:  Julian Rüth  Reviewers:  Adele Bourgeois, Maarten Derickx 
Report Upstream:  N/A  Work issues:  
Branch:  8a1e18b (Commits, GitHub, GitLab)  Commit:  8a1e18b968a830f5d816723b0c7c3e530032ba06 
Dependencies:  #23485, #23484, #23483, #23482  Stopgaps: 
Description (last modified by )
This should return True
sage: R.<x> = ZZ[] sage: S.<x> = QQ[] sage: S.quo(x^2 + 1).coerce_map_from(R.quo(x^2 + 1)).is_injective()
Generally, if R→S
is injective/surjective then the quotient is.
Change History (38)
comment:1 Changed 5 years ago by
Description:  modified (diff) 

comment:2 Changed 5 years ago by
Summary:  Improve is_injective()/in_surjective() for coercions of quotient rings → Improve is_injective()/is_surjective() for coercions of quotient rings 

comment:3 Changed 5 years ago by
Keywords:  sd87 added 

comment:4 Changed 5 years ago by
Branch:  → u/saraedum/improve_is_injective___is_surjective___for_coercions_of_quotient_rings 

comment:5 Changed 5 years ago by
Branch:  u/saraedum/improve_is_injective___is_surjective___for_coercions_of_quotient_rings → u/saraedum/23190 

comment:6 Changed 5 years ago by
Commit:  → 64776a9e28ccdbc439dace9c43068c287ad95acd 

comment:7 Changed 5 years ago by
Dependencies:  → #23485, #23484, #23483, #23482 

comment:8 Changed 5 years ago by
Status:  new → needs_review 

comment:9 Changed 5 years ago by
Keywords:  beginner added 

comment:10 Changed 5 years ago by
comment:11 Changed 5 years ago by
Reviewers:  → Adele Bourgeois 

comment:12 Changed 5 years ago by
Authors:  → Julian Rüth 

comment:13 Changed 5 years ago by
Work issues:  → failing doctest 

comment:14 Changed 5 years ago by
Commit:  64776a9e28ccdbc439dace9c43068c287ad95acd → d27262a25f9b75220f3aa84d0e1fbfadb0744331 

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

comment:15 Changed 5 years ago by
Work issues:  failing doctest 

comment:17 Changed 5 years ago by
Replying to chapoton:
 missing enpty line after
EXAMPLES::
Is this fixed already? I could not find the EXAMPLES::
you are referring to.
comment:19 Changed 5 years ago by
Commit:  d27262a25f9b75220f3aa84d0e1fbfadb0744331 → 46cb2f90e1e534b51086f9b07f94854674307195 

Branch pushed to git repo; I updated commit sha1. New commits:
46cb2f9  Implement _richcmp_ as discussed in 23185

comment:20 Changed 5 years ago by
Milestone:  sage8.0 → sage8.1 

Status:  needs_review → needs_work 
does not apply
comment:21 Changed 5 years ago by
Commit:  46cb2f90e1e534b51086f9b07f94854674307195 → e2c1e36ad9e776a3069cdf56271f9d595637ecf0 

Branch pushed to git repo; I updated commit sha1. New commits:
e2c1e36  Merge branch 'develop' into t/23190/23190

comment:22 Changed 5 years ago by
Status:  needs_work → needs_review 

comment:23 Changed 5 years ago by
Status:  needs_review → needs_work 

Work issues:  → failing doctest 
comment:24 Changed 5 years ago by
File "src/sage/rings/polynomial/polynomial_quotient_ring.py", line 1868, in sage.rings.polynomial.polynomial_quotient_ring.PolynomialQuotientRing_coercion.is_surjective Failed example: f.is_surjective() Exception raised: Traceback (most recent call last): File "/tmp/sagepatchbot/sage/local/lib/python2.7/sitepackages/sage/doctest/forker.py", line 509, in _run self.compile_and_execute(example, compiler, test.globs) File "/tmp/sagepatchbot/sage/local/lib/python2.7/sitepackages/sage/doctest/forker.py", line 872, in compile_and_execute exec(compiled, globs) File "<doctest sage.rings.polynomial.polynomial_quotient_ring.PolynomialQuotientRing_coercion.is_surjective[2]>", line 1, in <module> f.is_surjective() File "/tmp/sagepatchbot/sage/local/lib/python2.7/sitepackages/sage/rings/polynomial/polynomial_quotient_ring.py", line 1882, in is_surjective constant_map_is_surjective = self.codomain().base_ring().coerce_map_from(self.domain().base_ring()).is_surjective() File "sage/categories/map.pyx", line 1204, in sage.categories.map.Map.is_surjective (build/cythonized/sage/categories/map.c:9316) raise NotImplementedError(type(self)) NotImplementedError: <type 'sage.categories.morphism.IdentityMorphism'>
comment:25 Changed 5 years ago by
Commit:  e2c1e36ad9e776a3069cdf56271f9d595637ecf0 → 944e05e05568202eebbcbe98bdb64beab39cdbe9 

Branch pushed to git repo; I updated commit sha1. New commits:
944e05e  Merge branch 'develop' into t/23190/23190

comment:26 Changed 5 years ago by
Status:  needs_work → needs_review 

Work issues:  failing doctest 
Somehow #23482 is not in develop yet, that's why the tests fail.
comment:27 Changed 5 years ago by
Status:  needs_review → needs_work 

I think the mathematical content of the statement if R > S is injective then the quotient is, is no true. A counter example is for example
ZZ['x'].quo(2) == GF(2)['x']
which does not inject into QQ['x'].quo(2) == 0
. This example is not a pathology of 2 being in the constant field, it fails for any polynomial whose content is not a unit.
A nice sufficient condition for injectivity is that the polynomial be monic (or more generally if its leading coefficient is a unit).
There are no problems with surjectivity.
comment:28 Changed 5 years ago by
Status:  needs_work → needs_review 

Thanks for catching that one. You are right of course. Fixed now.
comment:29 Changed 5 years ago by
Commit:  944e05e05568202eebbcbe98bdb64beab39cdbe9 → 19aee539f2e5ba1c65f408d9a5ee40eb59e3e0e3 

Branch pushed to git repo; I updated commit sha1. New commits:
19aee53  Fix is_injective

comment:30 Changed 5 years ago by
Commit:  19aee539f2e5ba1c65f408d9a5ee40eb59e3e0e3 → 282ea209b388f2fe76b4e93f192f74e7a749bf23 

Branch pushed to git repo; I updated commit sha1. New commits:
282ea20  Do not test is_jnjective() for nonunit leading coefficient

comment:31 Changed 5 years ago by
Ok the code looks fine now. If you can rewrite the extremely long lines such as:
if self.domain().modulus().change_ring(self.codomain().base_ring()) == self.codomain().modulus() and self.domain().modulus().leading_coefficient().is_unit():
in such a way that they comfortably fit on a reasonable screen and all the doctests pass. Then this ticket gets a positive review from me.
P.s. In reviewing this ticket I also found some mathematically incorrect behaviour of quotients of univariate polynomials rings. I created a separate ticket for this: #23621
comment:32 Changed 5 years ago by
Commit:  282ea209b388f2fe76b4e93f192f74e7a749bf23 → 8a1e18b968a830f5d816723b0c7c3e530032ba06 

Branch pushed to git repo; I updated commit sha1. New commits:
8a1e18b  break long lines

comment:33 Changed 5 years ago by
I only found that one line that's extremely long. For the others that are somewhat long I don't think there is a good position that calls for breaking them there. (But if you insist, I can break them of course.)
New commits:
8a1e18b  break long lines

comment:34 Changed 5 years ago by
Reviewers:  Adele Bourgeois → Adele Bourgeois, Maarten Derickx 

comment:35 followup: 36 Changed 5 years ago by
a question to abourgeois : should we write "Adele" or "Adèle" ?
comment:36 Changed 5 years ago by
Replying to chapoton:
a question to abourgeois : should we write "Adele" or "Adèle" ?
I usually sign my name with the accent, but I don't really have a preference in writing (some of my legal documents omit the accent).
comment:38 Changed 5 years ago by
Branch:  u/saraedum/23190 → 8a1e18b968a830f5d816723b0c7c3e530032ba06 

Resolution:  → fixed 
Status:  positive_review → closed 
Branch pushed to git repo; I updated commit sha1. New commits:
Implement is_injective/is_surjective for coercions of quotient rings