Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve is_injective()/is_surjective() for coercions of quotient rings #23190

Closed
saraedum opened this issue Jun 9, 2017 · 44 comments
Closed

Comments

@saraedum
Copy link
Member

saraedum commented Jun 9, 2017

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.

Depends on #23485
Depends on #23484
Depends on #23483
Depends on #23482

Component: commutative algebra

Keywords: sd86.5, sd87

Author: Julian Rüth

Branch/Commit: 8a1e18b

Reviewer: Adele Bourgeois, Maarten Derickx

Issue created by migration from https://trac.sagemath.org/ticket/23190

@saraedum

This comment has been minimized.

@saraedum saraedum changed the title Improve is_injective()/in_surjective() for coercions of quotient rings Improve is_injective()/is_surjective() for coercions of quotient rings Jul 14, 2017
@roed314
Copy link
Contributor

roed314 commented Jul 17, 2017

Changed keywords from sd86.5 to sd86.5, sd87

@saraedum
Copy link
Member Author

@saraedum
Copy link
Member Author

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 19, 2017

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

64776a9Implement is_injective/is_surjective for coercions of quotient rings

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 19, 2017

Commit: 64776a9

@saraedum
Copy link
Member Author

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

@sagetrac-abourgeois
Copy link
Mannequin

sagetrac-abourgeois mannequin commented Jul 20, 2017

comment:10

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.

@sagetrac-abourgeois
Copy link
Mannequin

sagetrac-abourgeois mannequin commented Jul 20, 2017

Reviewer: Adele Bourgeois

@saraedum
Copy link
Member Author

Author: Julian Rüth

@saraedum
Copy link
Member Author

Work Issues: failing doctest

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 23, 2017

Changed commit from 64776a9 to d27262a

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 23, 2017

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

d27262aFix _richcmp_

@saraedum
Copy link
Member Author

Changed work issues from failing doctest to none

@saraedum
Copy link
Member Author

New commits:

d27262aFix _richcmp_

New commits:

d27262aFix _richcmp_

@fchapoton
Copy link
Contributor

comment:16
  • missing enpty line after EXAMPLES::

@saraedum
Copy link
Member Author

comment:17

Replying to @fchapoton:

  • missing enpty line after EXAMPLES::

Is this fixed already? I could not find the EXAMPLES:: you are referring to.

@fchapoton
Copy link
Contributor

comment:18

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

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 27, 2017

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

46cb2f9Implement `_richcmp_` as discussed in 23185

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 27, 2017

Changed commit from d27262a to 46cb2f9

@fchapoton
Copy link
Contributor

comment:20

does not apply

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 8, 2017

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

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

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 8, 2017

Changed commit from e2c1e36 to 944e05e

@saraedum
Copy link
Member Author

saraedum commented Aug 8, 2017

Changed work issues from failing doctest to none

@saraedum
Copy link
Member Author

saraedum commented Aug 8, 2017

comment:26

Somehow #23482 is not in develop yet, that's why the tests fail.

@koffie
Copy link
Contributor

koffie commented Aug 13, 2017

comment:27

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.

@saraedum
Copy link
Member Author

comment:28

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

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 14, 2017

Changed commit from 944e05e to 19aee53

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 14, 2017

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

19aee53Fix is_injective

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 14, 2017

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

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

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 14, 2017

Changed commit from 19aee53 to 282ea20

@koffie
Copy link
Contributor

koffie commented Aug 14, 2017

comment:31

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

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 21, 2017

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

8a1e18bbreak long lines

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 21, 2017

Changed commit from 282ea20 to 8a1e18b

@saraedum
Copy link
Member Author

comment:33

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

@saraedum
Copy link
Member Author

Changed reviewer from Adele Bourgeois to Adele Bourgeois, Maarten Derickx

@fchapoton
Copy link
Contributor

comment:35

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

@sagetrac-abourgeois
Copy link
Mannequin

sagetrac-abourgeois mannequin commented Aug 21, 2017

comment:36

Replying to @fchapoton:

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).

@koffie
Copy link
Contributor

koffie commented Aug 30, 2017

comment:37

Ok, looks good to me.

@vbraun
Copy link
Member

vbraun commented Sep 10, 2017

Changed branch from u/saraedum/23190 to 8a1e18b

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants