Replace unfoldTreeM_BF and unfoldForestM_BF #126

Closed
wants to merge 2 commits into
from

Projects

None yet

1 participant

@treeowl
Member
treeowl commented Jan 1, 2015

Use radically simpler implementations that do not use
Data.Sequence.

@treeowl
Member
treeowl commented Jan 1, 2015

Do not merge (yet)! There are currently no QuickCheck properties for Data.Tree, and no Criterion benchmarks. I think this is probably right, and likely fast, but I don't know. Note that I used splitAlong to slightly enhance laziness. An alternative would be to fuse length calculations with the concatMap, which would likely perform differently, for better or worse, and would also have different space properties.

@treeowl treeowl Replace unfoldTreeM_BF and unfoldForestM_BF
Use radically simpler implementations that do not use
Data.Sequence.
45e5dfa
@treeowl
Member
treeowl commented Jan 1, 2015

I though I could maybe structure it better than this, but maybe not. Trying to avoid the pointless-looking concatMap followed by rebuild keeps running me into things that would make this O(n^2), which would not be cool. Maybe someone else can think of something, but this is the best I have for now.

@treeowl treeowl Make stricter
I think this should match the semantics of the current
implementation, but I'm a bit tired.
108ec3b
@treeowl
Member
treeowl commented Jan 1, 2015

The second commit is a stricter version. I think it matches the current semantics, and should perform better. All that assuming I got it right, which, considering how late it is, I may not have.

@treeowl
Member
treeowl commented Jan 1, 2015

It's morning, so I have a new idea that might get the laziness and the efficiency: in the concatMap "stream", insert markers indicating breaks at different levels. That way, rebuilding won't need lengths.

@treeowl
Member
treeowl commented Jan 1, 2015

Thinking aloud just a bit more, what I want rebuild to become is something akin to a parser for parenthesized expressions, but adjacent parentheses pointing the same direction get merged into each other and counted. The merging is required to avoid O(n^2) performance on trees in which each node has only one child.

@treeowl treeowl closed this Jan 1, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment