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

Trees with multiple roots #28

Merged
merged 15 commits into from
Sep 18, 2019
Merged

Trees with multiple roots #28

merged 15 commits into from
Sep 18, 2019

Conversation

rvanvenetie
Copy link
Owner

This implements the case that trees have multiple roots. It is done by adding a meta root to trees; this node holds a reference to all the actual roots.

@rvanvenetie rvanvenetie changed the base branch from master to spacetime September 17, 2019 18:44
root_tree_space = uniform_index_tree(2, 'x')
root = DebugDoubleNode((root_tree_time, root_tree_space))
left, right = root.refine(0)
meta_root_time = uniform_index_tree(2, 't', FakeHaarNode)
Copy link
Collaborator

Choose a reason for hiding this comment

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

kan je bij deze test case opschrijven wat 'ie meot testen? vooral die "with pyutest.raises" is voor mij niet zo duidelijk meer

Copy link
Owner Author

Choose a reason for hiding this comment

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

Okay, iets getypt.

assert [n.nodes[0].labda for n in sigma.bfs()] == [(0, 0), (0, 0), (0, 0)]
assert [n.nodes[1].labda for n in sigma.bfs()] == [(0, 0), (1, 0), (1, 1)]
assert [n.nodes[0].labda
for n in sigma.bfs(include_meta_root=False)] == [(0, 0), (0, 0),
Copy link
Collaborator

Choose a reason for hiding this comment

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

is het misschien natuurlijker om include_meta_root default op False te zetten? dan komen we die metaroots niet tegen in het BFS'en, tenzij we "er om vragen"

Copy link
Owner Author

Choose a reason for hiding this comment

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

Yes, gedaan <3

sigma.root.union(self.Lambda_in.project(0), i=0)
sigma.root.union(self.Lambda_out.project(1), i=1)
Copy link
Collaborator

Choose a reason for hiding this comment

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

wacht wacht waarom ook deze tweede projectie er in gooien? zodat we de meta-roots hebben gebouwd zeker?

Copy link
Owner Author

@rvanvenetie rvanvenetie Sep 18, 2019

Choose a reason for hiding this comment

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

Ja zie comment erboven.

Copy link
Collaborator

@Jannertje Jannertje left a comment

Choose a reason for hiding this comment

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

Ziet er goed uit, de Sigma-code wordt zowaar netter van deze change haha. Ik zie nog niet direct of dit helemaal goed gaat en of we de juiste dingen testen -- misschien nog een multiple-parent test?

@rvanvenetie
Copy link
Owner Author

PTAL

Python/spacetime/tree.py Outdated Show resolved Hide resolved
@rvanvenetie rvanvenetie merged commit cc70943 into spacetime Sep 18, 2019
@rvanvenetie rvanvenetie deleted the spacetime-multiple-roots branch September 18, 2019 12:31
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.

2 participants