Opened 4 years ago

Closed 4 years ago

#19036 closed enhancement (fixed)

Use vectors instead of tuples in Polyomino

Reported by: vdelecroix Owned by:
Priority: major Milestone: sage-7.2
Component: combinatorics Keywords:
Cc: slabbe Merged in:
Authors: Vincent Delecroix Reviewers: Sébastien Labbé
Report Upstream: N/A Work issues:
Branch: fb8f324 (Commits) Commit: fb8f324fa9e3cf16d22e3b91b8ee85e1f4aa6af9
Dependencies: Stopgaps:

Description

As noticed in #18987, polyominoes would be much faster if they would store (immutable) integer vectors instead of tuples.

Change History (6)

comment:1 Changed 4 years ago by vdelecroix

  • Authors set to Vincent Delecroix
  • Branch set to u/vdelecroix/19036
  • Commit set to 10f03e0cf2e11cf61785778d55c8a605d417a437
  • Dependencies #18987 deleted
  • Status changed from new to needs_review

New commits:

10f03e0Trac 19036: use vectors instead of tuples in polyomino

comment:2 Changed 4 years ago by vdelecroix

  • Milestone changed from sage-6.9 to sage-7.1

comment:3 Changed 4 years ago by slabbe

  • Status changed from needs_review to needs_work

The patchbot reports doctests failures in file quatumino:

**********************************************************************
2 items had failures:
   4 of  21 in sage.games.quantumino
   2 of  13 in sage.games.quantumino.QuantuminoSolver.solve
    [76 tests, 6 failures, 20.14 s]
----------------------------------------------------------------------
sage -t --long src/sage/games/quantumino.py  # 6 doctests failed
----------------------------------------------------------------------

comment:4 Changed 4 years ago by slabbe

  • Branch changed from u/vdelecroix/19036 to public/19036
  • Commit changed from 10f03e0cf2e11cf61785778d55c8a605d417a437 to fb8f324fa9e3cf16d22e3b91b8ee85e1f4aa6af9
  • Reviewers set to Sébastien Labbé
  • Status changed from needs_work to needs_review

I confirm this branch makes things faster:

BEFORE:

sage: from sage.games.quantumino import QuantuminoSolver 
sage: q = QuantuminoSolver(0)                            
sage: %timeit L = q.tiling_solver().rows() 
1 loop, best of 3: 7.26 s per loop                                      

AFTER:

sage: from sage.games.quantumino import QuantuminoSolver
sage: q = QuantuminoSolver(0)                                                 
sage: %timeit L = q.tiling_solver().rows()
1 loop, best of 3: 1.78 s per loop     

I fixed the long time doctests errors. To me this is a positive review. I let Vincent change the status to positive review if he agrees with my commit.

Note: I rebased the branch of Vincent on top of most recent development version.


New commits:

22e1d10Trac 19036: use vectors instead of tuples in polyomino
fb8f324trac #19036: fixing long time doctests

comment:5 Changed 4 years ago by vdelecroix

  • Milestone changed from sage-7.1 to sage-7.2
  • Status changed from needs_review to positive_review

comment:6 Changed 4 years ago by vbraun

  • Branch changed from public/19036 to fb8f324fa9e3cf16d22e3b91b8ee85e1f4aa6af9
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.