#23482 closed enhancement (fixed)

Identities are injective and surjective

Reported by: saraedum Owned by:
Priority: minor Milestone: sage-8.1
Component: algebra Keywords: sd87, beginner
Cc: Merged in:
Authors: Julian Rüth, Travis Scrimshaw Reviewers: Claire Tomesch, Travis Scrimshaw, Julian Rüth
Report Upstream: N/A Work issues:
Branch: 5a3e684 (Commits) Commit: 5a3e684d060c68da916471acc34ef6b9d3a40f30
Dependencies: Stopgaps:

Description


Change History (17)

comment:1 Changed 21 months ago by saraedum

  • Branch set to u/saraedum/identities_are_injective_and_surjective

comment:2 Changed 21 months ago by saraedum

  • Commit set to b283a1a3e777325e68984890f0ea127cc604d193
  • Status changed from new to needs_review

New commits:

b283a1aIdentity morphisms are injective and surjective

comment:3 Changed 21 months ago by saraedum

  • Keywords sd87 added

comment:4 Changed 21 months ago by saraedum

  • Keywords beginner added

comment:5 Changed 21 months ago by cmt

  • Reviewers set to cmt

comment:6 Changed 21 months ago by cmt

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

comment:7 Changed 21 months ago by cmt

  • Status changed from needs_review to needs_work

comment:8 Changed 21 months ago by saraedum

  • Dependencies set to #23204
  • Status changed from needs_work to needs_review

You are absolutely right. Thanks for pointing this out.

comment:9 Changed 21 months ago by cmt

  • Status changed from needs_review to positive_review

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

comment:10 Changed 21 months ago by saraedum

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

comment:11 Changed 21 months ago by saraedum

  • Reviewers cmt deleted

comment:12 Changed 21 months ago by tscrim

  • Status changed from positive_review to needs_work

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

comment:13 Changed 21 months ago by saraedum

  • Reviewers set to Claire Tomesch
  • Status changed from needs_work to positive_review

comment:14 Changed 21 months ago by tscrim

  • Branch changed from u/saraedum/identities_are_injective_and_surjective to public/algebra/identities_bijective-23482
  • Commit changed from b283a1a3e777325e68984890f0ea127cc604d193 to 5a3e684d060c68da916471acc34ef6b9d3a40f30
  • Component changed from coercion to algebra
  • Dependencies #23204 deleted
  • Priority changed from trivial to minor
  • Reviewers changed from Claire Tomesch to Claire Tomesch, Travis Scrimshaw
  • Status changed from positive_review to needs_review

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.

comment:15 Changed 21 months ago by saraedum

  • Authors changed from Julian Rüth to Julian Rüth, Travis Scrimshaw
  • Reviewers changed from Claire Tomesch, Travis Scrimshaw to Claire Tomesch, Travis Scrimshaw, Julian Rüth
  • Work issues set to waiting for the patchbot → positive review

comment:16 Changed 21 months ago by saraedum

  • Status changed from needs_review to positive_review
  • Work issues waiting for the patchbot → positive review deleted

comment:17 Changed 21 months ago by vbraun

  • Branch changed from public/algebra/identities_bijective-23482 to 5a3e684d060c68da916471acc34ef6b9d3a40f30
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.