Opened 11 years ago

Closed 10 years ago

#9109 closed enhancement (fixed)

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: Merged in: sage-4.7.alpha5
Authors: Florent Hivert Reviewers: Mike Hansen, Nicolas M. Thiéry
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description (last modified by nthiery)

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

Dependencies: #8702

Attachments (2)

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

Download all attachments as: .zip

Change History (14)

comment:1 Changed 11 years ago by hivert

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

comment:2 Changed 11 years ago by hivert

  • Description modified (diff)

comment:3 Changed 11 years ago by hivert

  • Description modified (diff)

comment:4 Changed 10 years ago by hivert

  • Owner changed from sage-combinat to hivert

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

Changed 10 years ago by hivert

comment:5 Changed 10 years ago by hivert

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

This should now be ready for review

comment:6 Changed 10 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 10 years ago by hivert

  • Description modified (diff)

comment:8 Changed 10 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: Changed 10 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 10 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 10 years ago by hivert

comment:11 Changed 10 years ago by nthiery

  • Status changed from needs_review to positive_review

comment:12 Changed 10 years ago by jdemeyer

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