Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add method to find a surrounding of a polyomino with isometric copies of itself #29160

Closed
seblabbe opened this issue Feb 6, 2020 · 21 comments
Closed

Comments

@seblabbe
Copy link
Contributor

seblabbe commented Feb 6, 2020

This ticket adds some code based on the tiling solver class and dancing links solver to find a surrounding of a polyomino with copies of itself. An example is below:

sage: from sage.combinat.tiling import Polyomino
sage: H = Polyomino([(-1, 1), (-1, 4), (-1, 7), (0, 0), (0, 1), (0, 2),
....: (0, 3), (0, 4), (0, 5), (0, 6), (0, 7), (0, 8), (1, 1), (1, 2),
....: (1, 3), (1, 4), (1, 5), (1, 6), (1, 7), (1, 8), (2, 0), (2, 2),
....: (2, 3), (2, 5), (2, 6), (2, 8)])
sage: H.show2d()

sage: %time solution = H.self_surrounding(10, ncpus=8)
CPU times: user 1.69 s, sys: 1.08 s, total: 2.77 s
Wall time: 3.85 s
sage: G = sum([p.show2d() for p in solution], Graphics())
sage: G

See: https://user-images.githubusercontent.com/2339795/216875696-aef5396f-e30f-4e85-b4f5-85f5a841b797.pngeesch%27s_problem

Component: combinatorics

Author: Sébastien Labbé

Branch/Commit: 9d36ca5

Reviewer: Travis Scrimshaw, Frédéric Chapoton

Issue created by migration from https://trac.sagemath.org/ticket/29160

@seblabbe seblabbe added this to the sage-9.1 milestone Feb 6, 2020
@seblabbe
Copy link
Contributor Author

seblabbe commented Feb 6, 2020

comment:1

Attachment: H.png

@seblabbe

This comment has been minimized.

@seblabbe
Copy link
Contributor Author

seblabbe commented Feb 6, 2020

Attachment: G.png

@seblabbe
Copy link
Contributor Author

seblabbe commented Feb 6, 2020

New commits:

4b9e10dfinding a surrounding of a polyomino with copies of itself
402f1c229160: adding random colors to output

@seblabbe
Copy link
Contributor Author

seblabbe commented Feb 6, 2020

Branch: u/slabbe/29160

@seblabbe
Copy link
Contributor Author

seblabbe commented Feb 6, 2020

Commit: 402f1c2

@seblabbe

This comment has been minimized.

@tscrim
Copy link
Collaborator

tscrim commented Feb 8, 2020

comment:4

A few little things:

     - ``dimension`` -- integer (default: ``None``), dimension of the space, 
       if ``None``, it is guessed from the ``coords`` if ``coords`` is non
-      empty.
+      empty
                 raise ValueError("dimension(={}) must be provided for"
-                             " the empty polyomino".format(dimension))
+                                 " the empty polyomino".format(dimension))
-Return whether self is strictly inside of other.
+Return whether ``self`` is strictly inside of ``other``.

and other similar changes.

         OUTPUT:
 
-            polyomino
+        polyomino
         return Polyomino(self.frozenset() & other.frozenset(),
-                color=self._color, dimension=self._dimension)
+                         color=self._color, dimension=self._dimension)
         OUTPUT:
 
-            set of 3d polyominoes
+        set of 3d polyominoes
-        Using a Polyomino as input ::
+        Using a Polyomino as input::
-        - ``orientation_preserving`` -- bool (optional, default: ``True``),
-          If True, the group of isometries of the `n`-cube is restricted to
-          those that preserve the orientation, i.e. of determinant 1.
+        - ``orientation_preserving`` -- bool (optional, default: ``True``);
+          if ``True``, the group of isometries of the `n`-cube is restricted
+          to those that preserve the orientation, i.e. of determinant 1
-        S = set()
-        for cano in all_distinct_cano:
-            for t in cano.translated_copies_intersection(box=box):
-                S.add(t)
-        return S
+        return set([t for cano in all_distinct_cano
+                    for t in cano.translated_copies_intersection(box=box)])
     def self_surrounding(self, radius, remove_incomplete_copies=True,
-            ncpus=None):
+                         ncpus=None):
         OUTPUT:
 
-            list of polyominoes
+        list of polyominoes
            raise Exception('No solution was found with radius={}, '
            'this tile can not be surrounded by itself'.format(radius))

I don't like the fact that this raises a general Exception. I think it should be more specific like a ValueError.

Some of these are PEP8 and could amount to more of bikeshedding, but I think it is good to try and follow them. There are a few other such PEP8 code changes that could be done too beyond those I mentioned here.

@seblabbe
Copy link
Contributor Author

comment:5

Thanks Travis for your careful reading. I agree with your comments. I will work on it.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 15, 2020

Changed commit from 402f1c2 to 9cf100b

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 15, 2020

Branch pushed to git repo; I updated commit sha1. New commits:

9cf100b29160: style improvements (PEP8-like) after review

@seblabbe
Copy link
Contributor Author

comment:8

Each time, I searched and fixed similar issues in the same file.

@mkoeppe
Copy link
Member

mkoeppe commented Apr 14, 2020

comment:9

Batch modifying tickets that will likely not be ready for 9.1, based on a review of the ticket title, branch/review status, and last modification date.

@mkoeppe mkoeppe modified the milestones: sage-9.1, sage-9.2 Apr 14, 2020
@fchapoton
Copy link
Contributor

comment:10

La patchbot se plaint un petit peu :

+src/sage/combinat/tiling.py:446:15: E701 multiple statements on one line (colon)

et ca serait gentil de briser cette ligne en deux.

@seblabbe
Copy link
Contributor Author

comment:11

ok, je fais ça la semaine prochaine.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 13, 2020

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

66b2f8bfinding a surrounding of a polyomino with copies of itself
0c8bd4729160: adding random colors to output
fcfe62f29160: style improvements (PEP8-like) after review
9d36ca529160:one statement per line

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 13, 2020

Changed commit from 9cf100b to 9d36ca5

@seblabbe
Copy link
Contributor Author

comment:13

J'ai trouvé le double statement sur une même ligne. Je l'ai mis sur deux lignes séparées.

J'ai fait un rebase sur une version plus récente de Sage que j'utilise sur cette machine (9.1.rc1).

Needs review.

Sébastien

@seblabbe
Copy link
Contributor Author

Reviewer: Travis Scrimshaw, Frédéric Chapoton

@fchapoton
Copy link
Contributor

comment:15

ok, allons-y

@vbraun
Copy link
Member

vbraun commented Jul 19, 2020

Changed branch from u/slabbe/29160 to 9d36ca5

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants