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: sage-8.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:

Status badges

Description (last modified by Julian Rüth)

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 Julian Rüth

Description: modified (diff)

comment:2 Changed 5 years ago by Julian Rüth

Summary: Improve is_injective()/in_surjective() for coercions of quotient ringsImprove is_injective()/is_surjective() for coercions of quotient rings

comment:3 Changed 5 years ago by David Roe

Keywords: sd87 added

comment:4 Changed 5 years ago by Julian Rüth

Branch: u/saraedum/improve_is_injective___is_surjective___for_coercions_of_quotient_rings

comment:5 Changed 5 years ago by Julian Rüth

Branch: u/saraedum/improve_is_injective___is_surjective___for_coercions_of_quotient_ringsu/saraedum/23190

comment:6 Changed 5 years ago by git

Commit: 64776a9e28ccdbc439dace9c43068c287ad95acd

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

64776a9Implement is_injective/is_surjective for coercions of quotient rings

comment:7 Changed 5 years ago by Julian Rüth

Dependencies: #23485, #23484, #23483, #23482

comment:8 Changed 5 years ago by Julian Rüth

Status: newneeds_review

comment:9 Changed 5 years ago by Julian Rüth

Keywords: beginner added

comment:10 Changed 5 years ago by Adèle Bourgeois

All doctests pass on src/sage/rings/polynomial/polynomial_quotient_ring.py However, when running the doctests on src/sage/rings, I believe it requires #23204, based on the dependencies of #23482 and #23483.

comment:11 Changed 5 years ago by Adèle Bourgeois

Reviewers: Adele Bourgeois

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

Authors: Julian Rüth

comment:13 Changed 5 years ago by Julian Rüth

Work issues: failing doctest

comment:14 Changed 5 years ago by git

Commit: 64776a9e28ccdbc439dace9c43068c287ad95acdd27262a25f9b75220f3aa84d0e1fbfadb0744331

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

d27262aFix _richcmp_

comment:15 Changed 5 years ago by Julian Rüth

Work issues: failing doctest

New commits:

d27262aFix _richcmp_

New commits:

d27262aFix _richcmp_

comment:16 Changed 5 years ago by Frédéric Chapoton

  • missing enpty line after EXAMPLES::

comment:17 in reply to:  16 Changed 5 years ago by Julian Rüth

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 5 years ago by Frédéric Chapoton

indeed. But there is the same issue in richcmp as in #23185

comment:19 Changed 5 years ago by git

Commit: d27262a25f9b75220f3aa84d0e1fbfadb074433146cb2f90e1e534b51086f9b07f94854674307195

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

46cb2f9Implement _richcmp_ as discussed in 23185

comment:20 Changed 5 years ago by Frédéric Chapoton

Milestone: sage-8.0sage-8.1
Status: needs_reviewneeds_work

does not apply

comment:21 Changed 5 years ago by git

Commit: 46cb2f90e1e534b51086f9b07f94854674307195e2c1e36ad9e776a3069cdf56271f9d595637ecf0

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

e2c1e36Merge branch 'develop' into t/23190/23190

comment:22 Changed 5 years ago by Julian Rüth

Status: needs_workneeds_review

comment:23 Changed 5 years ago by Julian Rüth

Status: needs_reviewneeds_work
Work issues: failing doctest

comment:24 Changed 5 years ago by Julian Rüth

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/sage-patchbot/sage/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 509, in _run
        self.compile_and_execute(example, compiler, test.globs)
      File "/tmp/sage-patchbot/sage/local/lib/python2.7/site-packages/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/sage-patchbot/sage/local/lib/python2.7/site-packages/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 git

Commit: e2c1e36ad9e776a3069cdf56271f9d595637ecf0944e05e05568202eebbcbe98bdb64beab39cdbe9

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

944e05eMerge branch 'develop' into t/23190/23190

comment:26 Changed 5 years ago by Julian Rüth

Status: needs_workneeds_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 Maarten Derickx

Status: needs_reviewneeds_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.

Last edited 5 years ago by Maarten Derickx (previous) (diff)

comment:28 Changed 5 years ago by Julian Rüth

Status: needs_workneeds_review

Thanks for catching that one. You are right of course. Fixed now.

comment:29 Changed 5 years ago by git

Commit: 944e05e05568202eebbcbe98bdb64beab39cdbe919aee539f2e5ba1c65f408d9a5ee40eb59e3e0e3

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

19aee53Fix is_injective

comment:30 Changed 5 years ago by git

Commit: 19aee539f2e5ba1c65f408d9a5ee40eb59e3e0e3282ea209b388f2fe76b4e93f192f74e7a749bf23

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

282ea20Do not test is_jnjective() for non-unit leading coefficient

comment:31 Changed 5 years ago by Maarten Derickx

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

Last edited 5 years ago by Maarten Derickx (previous) (diff)

comment:32 Changed 5 years ago by git

Commit: 282ea209b388f2fe76b4e93f192f74e7a749bf238a1e18b968a830f5d816723b0c7c3e530032ba06

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

8a1e18bbreak long lines

comment:33 Changed 5 years ago by Julian Rüth

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:

8a1e18bbreak long lines
Last edited 5 years ago by Julian Rüth (previous) (diff)

comment:34 Changed 5 years ago by Julian Rüth

Reviewers: Adele BourgeoisAdele Bourgeois, Maarten Derickx

comment:35 Changed 5 years ago by Frédéric Chapoton

a question to abourgeois : should we write "Adele" or "Adèle" ?

comment:36 in reply to:  35 Changed 5 years ago by Adèle Bourgeois

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 5 years ago by Maarten Derickx

Status: needs_reviewpositive_review

Ok, looks good to me.

comment:38 Changed 5 years ago by Volker Braun

Branch: u/saraedum/231908a1e18b968a830f5d816723b0c7c3e530032ba06
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.