Opened 6 years ago
Closed 6 years ago
#21879 closed enhancement (fixed)
Implement is_injective for ring homomorphisms
Reported by:  Julian Rüth  Owned by:  

Priority:  major  Milestone:  sage7.5 
Component:  categories  Keywords:  Rings, injectivity, is_injective, MorphismMethods, is_subring 
Cc:  David Roe, Xavier Caruso  Merged in:  
Authors:  Julian Rüth  Reviewers:  David Roe 
Report Upstream:  N/A  Work issues:  
Branch:  2f641e2 (Commits, GitHub, GitLab)  Commit:  2f641e23f91a98b333ab106b69590d95a79e59e5 
Dependencies:  #21893, #19820, #21895  Stopgaps: 
Description (last modified by )
Currently, is_injective
is almost never implemented:
sage: ZZ.hom(QQ).is_injective() NotImplementedError sage: QQ.hom(QQ).is_injective() NotImplementedError
This makes the method is_subring
mostly useless since its default implementation relies on is_injective
of the "natural map":
sage: GF(3).is_subring(GF(9)) NotImplementedError
The changes introduced here, implement it as a method of the morphisms in the category of Rings where we can handle many common cases of morphisms (domain is a field, ….)
Note that is_injective
is not shown in the tab completion anymore. A problem that is fixed in #21880.
Change History (21)
comment:1 Changed 6 years ago by
Authors:  → Julian Rüth 

comment:2 Changed 6 years ago by
Description:  modified (diff) 

comment:3 Changed 6 years ago by
Description:  modified (diff) 

comment:4 Changed 6 years ago by
Branch:  → u/saraedum/implement_is_injective_for_ring_homomorphisms 

comment:5 Changed 6 years ago by
Commit:  → 1a5fac4f83cb9e09a81acce74e0857245b315e87 

Status:  new → needs_review 
comment:6 Changed 6 years ago by
Commit:  1a5fac4f83cb9e09a81acce74e0857245b315e87 → 689028182d31068228562360264e86af492aadf9 

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
6890281  Implement is_injective as a MorphismMethod

comment:7 Changed 6 years ago by
Note that is_injective
is now faster even for the one case where it just returned True
before.
sage: K.<x> = FunctionField(QQ) sage: f = K.hom(x) sage: %timeit f.is_injective()
Went down from 182ns to 76ns (which is of course due to the @cached_method
.) Without it, it goes up to 13µs.
comment:8 Changed 6 years ago by
Cc:  David Roe Xavier Caruso added 

comment:9 Changed 6 years ago by
Description:  modified (diff) 

comment:10 Changed 6 years ago by
Commit:  689028182d31068228562360264e86af492aadf9 → 363ece8ce62dc8cb7a510607d0f34778adc4b74f 

comment:11 Changed 6 years ago by
Commit:  363ece8ce62dc8cb7a510607d0f34778adc4b74f → 2f641e23f91a98b333ab106b69590d95a79e59e5 

Branch pushed to git repo; I updated commit sha1. New commits:
2f641e2  Add missing doctest output

comment:12 Changed 6 years ago by
Dependencies:  → #21893 

New commits:
2f641e2  Add missing doctest output

comment:13 Changed 6 years ago by
Reviewers:  → David Roe 

Status:  needs_review → positive_review 
Assuming that the patchbot is happy, I am.
comment:14 Changed 6 years ago by
Dependencies:  #21893 → #21893, #21894, #21895 

comment:15 Changed 6 years ago by
comment:16 Changed 6 years ago by
You should just figure out what you actually want to depend on. This ticket will not be merged until all dependencies are merged.
comment:17 Changed 6 years ago by
vbraun: I know. I will remove one of the dependencies as soon as the other one is merged.
comment:20 Changed 6 years ago by
Dependencies:  #21893, #21894, #21895 → #21893, #19820, #21895 

We do not need the full #21894. #19820 is enough to pass the doctests. Alternatively, the revert of https://git.sagemath.org/sage.git/commit?id=86b4868e9deb41940e3bed2a3289276bd7557a52 would also be enough but I think that #19820 is a more fundamental fix to the underlying problem (broken __eq__
implementations.)
comment:21 Changed 6 years ago by
Branch:  u/saraedum/implement_is_injective_for_ring_homomorphisms → 2f641e23f91a98b333ab106b69590d95a79e59e5 

Resolution:  → fixed 
Status:  positive_review → closed 
New commits:
fixed typo
Implement is_injective as a MorphismMethod