Ticket #9109 (closed enhancement: fixed)

Opened 3 years ago

Last modified 2 years ago

Fast cython class for maps between finite sets.

Reported by: hivert Owned by: hivert
Priority: major Milestone: sage-4.7
Component: combinatorics Keywords: finite sets map
Cc: Work issues:
Report Upstream: N/A Reviewers: Mike Hansen, Nicolas M. Thiéry
Authors: Florent Hivert Merged in: sage-4.7.alpha5
Dependencies: Stopgaps:

Description (last modified by nthiery) (diff)

The patch implements fast Cython classes for maps between finite sets. The goal in particular is to allow for building easily submonoids of the monoid of all maps from one set to itself.

See for example class NDPFMonoidPosetNewSet in  parking_functions_posets-fh.patch

Apply:

  1. trac_9109-finite_set_maps-fh.patch Download

Dependencies: #8702

Attachments

trac_9109-finite_set_maps-fh.2.patch Download (32.4 KB) - added by hivert 3 years ago.
trac_9109-finite_set_maps-fh.patch Download (41.5 KB) - added by hivert 2 years ago.

Change History

comment:1 Changed 3 years ago by hivert

  • Status changed from new to needs_review
  • Description modified (diff)

comment:2 Changed 3 years ago by hivert

  • Description modified (diff)

comment:3 Changed 3 years ago by hivert

  • Description modified (diff)

comment:4 Changed 3 years ago by hivert

  • Owner changed from sage-combinat to hivert

I just uploaded a new version after some changes in #8702

Changed 3 years ago by hivert

comment:5 Changed 2 years ago by hivert

  • Reviewers set to Mike Hansen, Nicolas M. Thiéry
  • Description modified (diff)

This should now be ready for review

comment:6 Changed 2 years ago by hivert

  • Status changed from needs_review to needs_work

They are still a few glitches remaining. Setting status back to need work

comment:7 Changed 2 years ago by hivert

  • Description modified (diff)

comment:8 Changed 2 years ago by hivert

  • Status changed from needs_work to needs_review

The submitted patch should fix all remaining problem. Please review.

comment:9 follow-up: ↓ 10 Changed 2 years ago by nthiery

  • Description modified (diff)

We discussed the design quite some with Florent, and used the code intensively over the last month. It's a useful addition! Thanks!

Florent: please check the small reviewer patch on the Sage-Combinat queue. If the changes are ok with you, you can fold/upload and set a positive review on my behalf.

comment:10 in reply to: ↑ 9 Changed 2 years ago by hivert

Florent: please check the small reviewer patch on the Sage-Combinat queue. If the changes are ok with you, you can fold/upload and set a positive review on my behalf.

Hi Nicolas ! Thanks a lot for the numerous English corrections. The doc is much better now ! I'm perfectly ok with all your changes, except for the following wrong ReST markup:

diff --git a/sage/sets/finite_set_map_cy.pyx b/sage/sets/finite_set_map_cy.pyx
--- a/sage/sets/finite_set_map_cy.pyx
+++ b/sage/sets/finite_set_map_cy.pyx
@@ -86,7 +86,7 @@ cpdef fibers(f, domain):
         {1: {1}}
 
     .. seealso:: :func:`fibers_args` if one needs to pass extra
-    arguments to ``f``.
+       arguments to ``f``.
     """
     result = {}
     for x in domain:

I folded your patch and added this small change. It is a rather trivial change but, in order to follow the rules, someone has to look at it. So it's now your job to set the positive review :-)

Changed 2 years ago by hivert

comment:11 Changed 2 years ago by nthiery

  • Status changed from needs_review to positive_review

comment:12 Changed 2 years ago by jdemeyer

  • Status changed from positive_review to closed
  • Resolution set to fixed
  • Merged in set to sage-4.7.alpha5
Note: See TracTickets for help on using tickets.