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

Identities are injective and surjective #23482

Closed
saraedum opened this issue Jul 19, 2017 · 23 comments
Closed

Identities are injective and surjective #23482

saraedum opened this issue Jul 19, 2017 · 23 comments

Comments

@saraedum
Copy link
Member

Component: algebra

Keywords: sd87

Author: Julian Rüth, Travis Scrimshaw

Branch/Commit: 5a3e684

Reviewer: Claire Tomesch, Travis Scrimshaw, Julian Rüth

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

@saraedum saraedum added this to the sage-8.1 milestone Jul 19, 2017
@saraedum
Copy link
Member Author

@saraedum
Copy link
Member Author

New commits:

b283a1aIdentity morphisms are injective and surjective

@saraedum
Copy link
Member Author

Commit: b283a1a

@saraedum
Copy link
Member Author

Changed keywords from none to sd87

@sagetrac-cmt
Copy link
Mannequin

sagetrac-cmt mannequin commented Jul 20, 2017

Reviewer: cmt

@sagetrac-cmt
Copy link
Mannequin

sagetrac-cmt mannequin commented Jul 20, 2017

comment:6

I checked out the ticket branch, ran ./sage -ba to rebuild all of Sage including all the Cython, and ran ./sage -tp 2 --long src/sage/categories/morphism.pyx to run the relevant doctests. However, one doctest failed, as the output below shows:

bash-3.2$ ./sage -tp 2 --long src/sage/categories/morphism.pyx
too few successful tests, not using stored timings
Running doctests with ID 2017-07-20-13-16-59-0bd01dfc.
Git branch: ticket_23482
Using --optional=mpir,python2,sage
Doctesting 1 file using 2 threads.
sage -t --long src/sage/categories/morphism.pyx
**********************************************************************
File "src/sage/categories/morphism.pyx", line 429, in sage.categories.morphism.IdentityMorphism.is_surjective
Failed example:
    ZZ.hom(ZZ).is_surjective()
Exception raised:
    Traceback (most recent call last):
      File "/Sage/sage/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 509, in _run
        self.compile_and_execute(example, compiler, test.globs)
      File "/Sage/sage/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 872, in compile_and_execute
        exec(compiled, globs)
      File "<doctest sage.categories.morphism.IdentityMorphism.is_surjective[0]>", line 1, in <module>
        ZZ.hom(ZZ).is_surjective()
      File "sage/categories/map.pyx", line 1204, in sage.categories.map.Map.is_surjective (/Sage/sage/src/build/cythonized/sage/categories/map.c:9227)
        raise NotImplementedError(type(self))
    NotImplementedError: <type 'sage.rings.morphism.RingHomomorphism_coercion'>
**********************************************************************
1 item had failures:
   1 of   2 in sage.categories.morphism.IdentityMorphism.is_surjective
    [105 tests, 1 failure, 0.67 s]
----------------------------------------------------------------------
sage -t --long src/sage/categories/morphism.pyx  # 1 doctest failed
----------------------------------------------------------------------
Total time for all tests: 0.7 seconds
    cpu time: 0.7 seconds
    cumulative wall time: 0.7 seconds

It seems like this failure is coming from the fact that the objects used in the doctests rely on RingHomomorphism_coercion, whose removal and replacement was proposed in [ticket:23204 #23204] but which this ticket isn't listed as being dependent on.

I am not sure what the owner of this ticket would prefer to do at this point.

@saraedum
Copy link
Member Author

comment:8

You are absolutely right. Thanks for pointing this out.

@saraedum
Copy link
Member Author

Dependencies: #23204

@sagetrac-cmt
Copy link
Mannequin

sagetrac-cmt mannequin commented Jul 20, 2017

comment:9

I reran ./sage -tp 2 --long src/sage/categories/morphism.pyx and now it returned that all tests passed!

@saraedum
Copy link
Member Author

comment:10

Great :) Please add your real name in the "Reviewer" field.

@saraedum
Copy link
Member Author

Changed reviewer from cmt to none

@tscrim
Copy link
Collaborator

tscrim commented Jul 20, 2017

comment:12

Setting to needs work because missing reviewer name.

However, I think this should be done independently of #23204, which just means writing a little smarter doctest:

sage: Hom(QQ, QQ).identity()
Identity endomorphism of Rational Field

@saraedum
Copy link
Member Author

Reviewer: Claire Tomesch

@tscrim
Copy link
Collaborator

tscrim commented Jul 21, 2017

@tscrim
Copy link
Collaborator

tscrim commented Jul 21, 2017

Changed reviewer from Claire Tomesch to Claire Tomesch, Travis Scrimshaw

@tscrim
Copy link
Collaborator

tscrim commented Jul 21, 2017

Changed commit from b283a1a to 5a3e684

@tscrim
Copy link
Collaborator

tscrim commented Jul 21, 2017

comment:14

Thinking more about this, it really is a good idea to not depend on #23204 and do a doctest that is a little more robust. My changes should be easy to review.


New commits:

5a3e684Use morphisms that are guaranteed to be the IdentityMorphism.

@tscrim
Copy link
Collaborator

tscrim commented Jul 21, 2017

Changed dependencies from #23204 to none

@saraedum
Copy link
Member Author

Changed reviewer from Claire Tomesch, Travis Scrimshaw to Claire Tomesch, Travis Scrimshaw, Julian Rüth

@saraedum
Copy link
Member Author

Work Issues: waiting for the patchbot → positive review

@saraedum
Copy link
Member Author

Changed author from Julian Rüth to Julian Rüth, Travis Scrimshaw

@saraedum
Copy link
Member Author

Changed work issues from waiting for the patchbot → positive review to none

@vbraun
Copy link
Member

vbraun commented Aug 5, 2017

Changed branch from public/algebra/identities_bijective-23482 to 5a3e684

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

3 participants