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

Obstruction inferral #61

Merged
merged 10 commits into from
Sep 17, 2019
Merged

Obstruction inferral #61

merged 10 commits into from
Sep 17, 2019

Conversation

enadeau
Copy link
Member

@enadeau enadeau commented Sep 13, 2019

Address issue #52

I'm still working on all obstruction inferral and the tiling methods.

@enadeau enadeau self-assigned this Sep 16, 2019
@enadeau
Copy link
Member Author

enadeau commented Sep 16, 2019

I'm done with the work on this. However, the implementation for all obstruction inferral is implemented very naively at the moment. Any suggestion to improve it is welcome. I might have one that we can discuss but I'm not sure it is worth our time at the moment.

tilings/tiling.py Outdated Show resolved Hide resolved
return all(any(gp not in req for req in req_list)
for req_list in self._tiling.requirements)

def potential_new_obs(self):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would suggest doing

yield from Tiling(obstructions).gridded_perms_of_length(length)

It should achieve the same thing with far less explosion.

You probably want to set the tiling initialiser flags to False.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that this suggestion doesn't work. Take the following code

t = Tiling(obstructions=[
    Obstruction(Perm((0, 1, 2)), ((0, 0),)*3),
    Obstruction(Perm((0, 1, 2)), ((1, 0),)*3),
    Obstruction(Perm((0, 1)), ((0, 0), (1, 0))),
    Obstruction(Perm((1, 0)), ((0, 0), (1, 0))),
], requirements=[[
    Requirement(Perm((0,)), ((1, 0),)),
]])
print(t)
print('Contained gridded permutations of length 1')
for p in t.gridded_perms_of_length(1):
    print(p)

The output is

+-+-+
|1|2|
+-+-+
1: Av(012)
2: Av+(012)
Crossing obstructions:
01: (0, 0), (1, 0)
10: (0, 0), (1, 0)
Requirement 0:
0: (1, 0)
Contained gridded permutations of length 1
0: (1, 0)

With your suggestion we would never try to add the obstruction 0: (0, 0)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We would, note, I am suggesting initialising without the requirements.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A thought:

Let T = (O, R) be a tiling, and T' = (O, emptyset).

Is Grid(T') - Grid(T) precisely those we want to add?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is obviously wrong... (e.g every one by one positive tiling becomes empty...).

However, all obstructions we can add do satisfy o in Grid(T') and o not in Grid(T).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I pushed a more clever filtering that includes what we've discussed

Copy link
Member

@ulfarsson ulfarsson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good.

@ulfarsson ulfarsson merged commit 39e811a into develop Sep 17, 2019
@ulfarsson ulfarsson deleted the sub-obs-inf branch September 17, 2019 12:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants