#11379: Add Quantamino solver to sage/games
-----------------------------+----------------------------------------------
Reporter: slabbe | Owner: slabbe
Type: enhancement | Status: needs_work
Priority: major | Milestone: sage-4.7.1
Component: PLEASE CHANGE | Keywords:
Work_issues: | Upstream: N/A
Reviewer: | Author:
Merged: | Dependencies:
-----------------------------+----------------------------------------------
Changes (by rbeezer):
* status: needs_review => needs_work
Comment:
Hi Sebastien,
Very nice - this is a lot of fun, and a great advertisement for the power
of dancing links. Lots of little comments, I hope its not too much.
Nothing very serious.
* One doctest failure. I'm on 64-bit Linux - could 32-bit/64-bit be
the cause?
{{{
sage -t "devel/sage-main/sage/games/quantumino.py"
**********************************************************************
File "/sage/sage-4.7/devel/sage-main/sage/games/quantumino.py", line 193:
sage: hash(p)
Expected:
2059134902
Got:
6915256369230374838
**********************************************************************
}}}
* Why does block number 8 (yellow) have a hole in the middle? See
discussion below about `size`.
* Documentation looks good. I would have liked to see a bit more
detail at the module level for a quickstart - more like for the
`QuantuminoSolver` class-level documentation. Specifically:
* Make it clearer that you will see the puzzle pieces in 3D with the
``show_pentaminos()`` command.
* Show how to get the list of the actual placements for a single
solution. I wanted to do this once I had a solution since the `_repr_`
was not as informative as my curiosity.
* Maybe a real quick overview of the classes: the pentamino and
polyomino classes, the solution class, and the solver class.
* I thought an interact would be fun. Checkboxes for the excluded
piece, plus the ability to "explode" the solution via a slider. I'm
'''not''' suggesting you write an interact, but a `size` argument as input
to `show3d()` (for the solution) would make this possible. Patch attempts
to do this, but it is not totally correct, at size below 0.50 the pieces
start to fall apart into cubes. And the aside piece breaks up even
earlier in my test. The hole in block number 8 behaves slightly
differently. So as a suggestion: consider adding a `size` argument to pass
from the solution `show3d()` down to each piece. But my patch is just a
suggestion - it is not ready to use.
* Some of the lesser methods could use some improved documentation. In
many cases, at least the first summary line, and/or the type of input.
For example:
* "Return the 3D polyomino defined by a set of 3d coordinates." It is
not clear what "defined by" means here, maybe the coordinates are for the
"bottommost" corner of each constituent cube.
* It is not clear what __sub__ does without looking at the code. A
one-line summary and an input list would be enough, I think.
* I'm not sure upper-case is part of Python or Sage style for code.
SPACE, COORD_TO_INT, and INT_TO_COORD look funny to my eye.
* I set up Sudoku puzzles with a "Sudoku" class. Then s =
Sudoku(.....), followed by s.solve() gave an iterator over solutions.
Would it be worth trying to mimic that approach for consistency? I'm not
arguing that my approach was better - just first. `:-)`
Minor editing
* "needs to creates" should be "create"
* "where each pieces is used exactly once" should be "piece"
* `# Class QuantuminoSolultion` (misspelled in comment)
* I think even with the utf-8 header the consensus (rule?) has been
straight ASCII in source files? Which I know even impacts your name. I
wish it wasn't this way (maybe I'm wrong?). The trademark symmbol and
bracketing on "think inside the box" for the game description would need
adjustments.
* `#bug trac #11272` - can this go away?
* `#return G` (twice) - can these go away?
As I said, lots of little stuff, which I hope does not look like too long
a list. I've tried to keep it to suggestions so you have the latitude to
approach it as you see fit. I'll be happy to stick with this review as
you make revisions.
Rob
--
Ticket URL: <http://trac.sagemath.org/sage_trac/ticket/11379#comment:5>
Sage <http://www.sagemath.org>
Sage: Creating a Viable Open Source Alternative to Magma, Maple, Mathematica,
and MATLAB
--
You received this message because you are subscribed to the Google Groups
"sage-trac" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to
[email protected].
For more options, visit this group at
http://groups.google.com/group/sage-trac?hl=en.
