Ticket #8691 (closed enhancement: fixed)

Opened 3 years ago

Last modified 3 years ago

Implement Plain Change algorithm in cython

Reported by: boothby Owned by: sage-combinat
Priority: minor Milestone: sage-4.4.4
Component: combinatorics Keywords:
Cc: rlm Work issues:
Report Upstream: N/A Reviewers: Robert Miller
Authors: Tom Boothby Merged in: sage-4.4.4.alpha0
Dependencies: Stopgaps:

Description

The implementation is already done, I just need a ticket number.

My fix for #8655 will depend on this.

Attachments

8691_permutation_plainchange_tjb.patch Download (8.7 KB) - added by boothby 3 years ago.
8691_permutation_plainchange_tjb_v2.patch Download (8.8 KB) - added by boothby 3 years ago.
replaces previous
8691_permutation_plainchange_tjb_v3.patch Download (4.1 KB) - added by boothby 3 years ago.
replaces all previous
8691_permutation_plainchange_tjb_v3_part2.patch Download (2.7 KB) - added by boothby 3 years ago.
apply on top of v3

Change History

Changed 3 years ago by boothby

comment:1 follow-up: ↓ 2 Changed 3 years ago by boothby

  • Status changed from new to needs_review

comment:2 in reply to: ↑ 1 ; follow-up: ↓ 3 Changed 3 years ago by hivert

Hi Tom,

This looks very good ! Thanks for this. I've only one concerns: from the name, I would never guess what it does. I'm not sure to have a better name but I think we should ask for better proposal on the mailing lists.

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

This looks very good ! Thanks for this. I've only one concerns: from the name, I would never guess what it does. I'm not sure to have a better name but I think we should ask for better proposal on the mailing lists.

Sorry for the double answer. Is you algorithm different from  http://en.wikipedia.org/wiki/Steinhaus–Johnson–Trotter_algorithm If not this is maybe a good name.

comment:4 Changed 3 years ago by boothby

  • Status changed from needs_review to needs_work

Oops, turns out that freeing a call to malloc(0) is a bad idea.

Changed 3 years ago by boothby

replaces previous

comment:5 Changed 3 years ago by boothby

  • Status changed from needs_work to needs_review

Changed 3 years ago by boothby

replaces all previous

comment:6 Changed 3 years ago by boothby

Updated version has changed the filename, and removed reference implementation because there's a better one in sage/graphs/genus.pyx.

comment:7 Changed 3 years ago by boothby

  • Cc rlm added

comment:8 Changed 3 years ago by rlm

I think the usual convention here is to have Python functions that can test low-level C functions. It would also check input, etc...

Changed 3 years ago by boothby

apply on top of v3

comment:9 Changed 3 years ago by boothby

  • Priority changed from major to minor
  • Type changed from defect to enhancement

comment:10 Changed 3 years ago by rlm

  • Status changed from needs_review to positive_review
  • Reviewers set to Robert Miller
  • Authors set to Tom Boothby

Looks good to me.

comment:11 Changed 3 years ago by mhansen

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