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

Transducer equivalent of tree-seq #20

Open
green-coder opened this issue Feb 19, 2018 · 9 comments
Open

Transducer equivalent of tree-seq #20

green-coder opened this issue Feb 19, 2018 · 9 comments

Comments

@green-coder
Copy link

I implemented this transducer, I am wondering if you would be interested to include it in your library.

https://gist.github.com/green-coder/3adf11660b7b0ca83648c5be69de2a3b

@cgrand
Copy link
Owner

cgrand commented Feb 19, 2018 via email

@ghadishayban
Copy link

make sure to test your reduced? cases. unreduced or ensure-reduced are your friends here

@green-coder
Copy link
Author

green-coder commented Feb 20, 2018

@cgrand I was not clear enough when I wrote my post this morning (at 2 am). Here is my second attempt:

I originally wanted to implement a transducer-only version of clojure.core.flatten, then someone pointed me to this gist which I found useful for bench-marking against other sequence-based implementations of clojure.core.tree-seq. Flatten relies on tree-seq so I went for a transducer-only version of tree-seq instead.

My function tree-seq'' is currently not using any sequence at all, the name is misleading. I use seq only to test if the children collection is empty. I could change (seq cs) into (empty? cs) but I am not sure if it is worth it as (empty? coll) is currently implemented as (not (seq coll)).

As suggested by @reborg, I need to change the name of the function. I am open to suggestions, but I will try to avoid name collision with some potential other tree-related transducers that I might add soon after. I am currently thinking to have one that only iterate on the tree leaves (i.e. the tree nodes that fail branch?), just like flatten.

@ghadishayban I will do additional tests, just in case. The function is not performing any early interruption of the transducing process, and it honors the next function rf if it does so. If you have some doubt or question about the implementation, please share them here.

@cgrand
Copy link
Owner

cgrand commented Feb 20, 2018 via email

@green-coder
Copy link
Author

I made the change. The function is smaller and the benchmark is showing that it is twice faster. That was a good feedback, thank you.

What do you think about tree-nodes for the name? I would like to also submit the version that only returns the leaves as tree-leaves.

;; This is a private function defined in `clojure.core`.
(defn preserving-reduced
  [rf]
  #(let [ret (rf %1 %2)]
     (if (reduced? ret)
       (reduced ret)
       ret)))

(defn tree-nodes
  ([branch? children]
   (fn [rf]
     (fn xf
       ([] (rf))
       ([result] (rf result))
       ([result input]
        (let [result (rf result input)]
          (if (reduced? result) result
            (if (branch? input)
              (reduce (preserving-reduced xf)
                      result
                      (children input))
              result))))))))

@cgrand
Copy link
Owner

cgrand commented Feb 20, 2018

Getting close. Minor grip: (preserving-reduced xf) may be evaluated several times.
Also these ifs could be collapsed in a cond.
I considered the name tree-cat myself.
I think there are more to think about when it comes to trees and transducers.

@green-coder
Copy link
Author

green-coder commented Feb 20, 2018

Coding style question: Would you prefer to have the (preserving-reduced xf) evaluated via a letfn before the level of the transducer or by simply having it expanded in-place?

I have no objection for tree-cat, but then how to call the leaf-only version? leaf-cat?

@cgrand
Copy link
Owner

cgrand commented Feb 20, 2018

Here's what I would do https://gist.github.com/cgrand/b5bf4851b0e5e3aeb438eba2298dacb9

(comp (tree-cat branch? children) (remove branch?)) is a good name for leaf-cat :-) except that branch? is going to be evaluated twice. We may find a way out by using key value pairs.

@scientific-coder
Copy link

Just came to say that I do think such a transducer would be useful. Am I wrong to believe that it could allow for elegant implementations of backtracking algorithms ? If I'm not mistaken, it could also be interesting to explore parallel execution as discussed in this great post about solving search problems with parallel depth first search in Clojure.

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

4 participants