Opened 2 years ago
Closed 23 months ago
#23190 closed enhancement (fixed)
Improve is_injective()/is_surjective() for coercions of quotient rings
Reported by:  saraedum  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)  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 2 years ago by
 Description modified (diff)
comment:2 Changed 2 years ago by
 Summary changed from Improve is_injective()/in_surjective() for coercions of quotient rings to Improve is_injective()/is_surjective() for coercions of quotient rings
comment:3 Changed 2 years ago by
 Keywords sd87 added
comment:4 Changed 2 years ago by
 Branch set to u/saraedum/improve_is_injective___is_surjective___for_coercions_of_quotient_rings
comment:5 Changed 2 years ago by
 Branch changed from u/saraedum/improve_is_injective___is_surjective___for_coercions_of_quotient_rings to u/saraedum/23190
comment:6 Changed 2 years ago by
 Commit set to 64776a9e28ccdbc439dace9c43068c287ad95acd
comment:7 Changed 2 years ago by
 Dependencies set to #23485, #23484, #23483, #23482
comment:8 Changed 2 years ago by
 Status changed from new to needs_review
comment:9 Changed 2 years ago by
 Keywords beginner added
comment:10 Changed 2 years ago by
comment:11 Changed 2 years ago by
 Reviewers set to Adele Bourgeois
comment:12 Changed 2 years ago by
comment:13 Changed 2 years ago by
 Work issues set to failing doctest
comment:14 Changed 2 years ago by
 Commit changed from 64776a9e28ccdbc439dace9c43068c287ad95acd to d27262a25f9b75220f3aa84d0e1fbfadb0744331
Branch pushed to git repo; I updated commit sha1. New commits:
d27262a  Fix _richcmp_

comment:15 Changed 2 years ago by
 Work issues failing doctest deleted
comment:16 followup: ↓ 17 Changed 2 years ago by
 missing enpty line after
EXAMPLES::
comment:17 in reply to: ↑ 16 Changed 2 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:18 Changed 2 years ago by
indeed. But there is the same issue in richcmp as in #23185
comment:19 Changed 2 years ago by
 Commit changed from d27262a25f9b75220f3aa84d0e1fbfadb0744331 to 46cb2f90e1e534b51086f9b07f94854674307195
Branch pushed to git repo; I updated commit sha1. New commits:
46cb2f9  Implement _richcmp_ as discussed in 23185

comment:20 Changed 2 years ago by
 Milestone changed from sage8.0 to sage8.1
 Status changed from needs_review to needs_work
does not apply
comment:21 Changed 2 years ago by
 Commit changed from 46cb2f90e1e534b51086f9b07f94854674307195 to e2c1e36ad9e776a3069cdf56271f9d595637ecf0
Branch pushed to git repo; I updated commit sha1. New commits:
e2c1e36  Merge branch 'develop' into t/23190/23190

comment:22 Changed 2 years ago by
 Status changed from needs_work to needs_review
comment:23 Changed 2 years ago by
 Status changed from needs_review to needs_work
 Work issues set to failing doctest
comment:24 Changed 2 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 2 years ago by
 Commit changed from e2c1e36ad9e776a3069cdf56271f9d595637ecf0 to 944e05e05568202eebbcbe98bdb64beab39cdbe9
Branch pushed to git repo; I updated commit sha1. New commits:
944e05e  Merge branch 'develop' into t/23190/23190

comment:26 Changed 2 years ago by
 Status changed from needs_work to needs_review
 Work issues failing doctest deleted
Somehow #23482 is not in develop yet, that's why the tests fail.
comment:27 Changed 2 years ago by
 Status changed from needs_review to 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 2 years ago by
 Status changed from needs_work to needs_review
Thanks for catching that one. You are right of course. Fixed now.
comment:29 Changed 2 years ago by
 Commit changed from 944e05e05568202eebbcbe98bdb64beab39cdbe9 to 19aee539f2e5ba1c65f408d9a5ee40eb59e3e0e3
Branch pushed to git repo; I updated commit sha1. New commits:
19aee53  Fix is_injective

comment:30 Changed 2 years ago by
 Commit changed from 19aee539f2e5ba1c65f408d9a5ee40eb59e3e0e3 to 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 2 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 2 years ago by
 Commit changed from 282ea209b388f2fe76b4e93f192f74e7a749bf23 to 8a1e18b968a830f5d816723b0c7c3e530032ba06
Branch pushed to git repo; I updated commit sha1. New commits:
8a1e18b  break long lines

comment:33 Changed 2 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 2 years ago by
 Reviewers changed from Adele Bourgeois to Adele Bourgeois, Maarten Derickx
comment:35 followup: ↓ 36 Changed 2 years ago by
a question to abourgeois : should we write "Adele" or "Adèle" ?
comment:36 in reply to: ↑ 35 Changed 2 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:37 Changed 23 months ago by
 Status changed from needs_review to positive_review
Ok, looks good to me.
comment:38 Changed 23 months ago by
 Branch changed from u/saraedum/23190 to 8a1e18b968a830f5d816723b0c7c3e530032ba06
 Resolution set to fixed
 Status changed from positive_review to closed
Branch pushed to git repo; I updated commit sha1. New commits:
Implement is_injective/is_surjective for coercions of quotient rings