Add comp, constantly and complement #1179

tuturto · 2016-12-19T12:23:44Z

relates #1176

gilch · 2016-12-19T22:04:52Z

It looks like comp is applying functions in the opposite order of Clojure's. Hy's comp should work the same as Clojure's comp.

Kodiologist · 2016-12-19T23:06:17Z

I agree that Clojure's order is the right one, since in mathematics, (g ∘ f)(x) usually means g(f(x)).

pyx · 2016-12-20T04:18:49Z

Agree with gilch and Kodiologist, ((comp f g) x) means (f (g x)).

Regarding tests, compose does obey three laws, provided function f, g and h are pure functions, id is identity function and .(dot) is compose function:

left identity
(id . f) x = f x
right identity
(f . id) x = f x
associative
((f . g) . h) x = (f . (g . h)) x

This is how I test compose in hymn, even though it looks like overkill for such a simple function. :)

tuturto · 2016-12-20T05:06:46Z

heh, that's rather embarrassing. I'll fix that. Good that it was spotted so soon.

Edit: actually, @pyx, can I just copy your implementation? It's concise and I don't think I can come up with anything like that. It's on a different license of course.

pyx · 2016-12-20T05:47:12Z

@tuturto, sure, my pleasure, if you find that useful, please do whatever you want, including re-licensing, I didn't put a no-evil clause on it. :D

pyx · 2016-12-20T06:26:01Z

@tuturto
In case you were talking about not only the tests but also the function itself, please notice that there is a design decision I made that might not be suitable for standard library: I did not catch the no-argument case, since the function was meant to be used only internally, an exception from reduce is enough for me.

I think there are two sensible options:

comp can be called with any number of arguments, return identity function when no argument provided.
EDIT: one if test for no-arugment case is enough, reduce can handle sequence with only one element.
comp should be called with at least one argument, something like

(defn compose [f &rest fs]
  "function composition"
  (defn compose-2 [f g]
    "compose 2 functions"
    (fn [&rest args &kwargs kwargs]
      (f (apply g args kwargs))))
  (reduce compose-2 fs f))

should do

tuturto · 2016-12-20T08:10:17Z

Thank you @pyx. I added the atleast-one-argument version and fixed docs while I was at it.

gilch · 2016-12-20T17:18:18Z

We should really follow Clojure's lead and choose option 1: return identity.

tuturto · 2016-12-20T21:40:27Z

I find it bit counter-intuitive that comp without any parameters returns identity, but it doesn't really make sense to deviate from other lisps. I added that there too now.

Kodiologist · 2016-12-20T21:42:31Z

The idea is that the identity function is the identity element for the composition operator. That is, (comp f identity) is equal to f. That's why it's the right choice for the nullary case.

tuturto · 2016-12-20T21:45:54Z

oh, hm.. If you put it that way it makes sense.

pyx · 2016-12-20T23:49:07Z

May I suggest a minor improvement,
since we have if taking out the empty case, and reduce can handle sequence with a single element by returning that sole element, which is what we expect in this case. (comp f) == f

(reduce compose-2 (list (rest fs)) (first fs))

can be

(reduce compose-2 fs)

gilch · 2016-12-21T22:39:16Z

Function calls are kind of expensive in Python I think it would be more efficient to use the imperative style, rather than calling a new function for every pair. Maybe something like this:

def comp(*fs):
    if not fs:
        return identity
    fs = reversed(fs)
    def composed(*args,**kwargs):
        for f in fs:
            res = f(*args,**kwargs)
            for f in fs:
                res = f(res)
        return res
    return composed

Kodiologist · 2016-12-21T22:45:13Z

@gilch I think a benchmark or example showing an improvement should be a minimum requirement for a performance-motivated change.

gilch · 2016-12-21T23:12:25Z

@Kodiologist, that's fair, but I'm starting to think Hy needs a timing macro. The timeit module is proving difficult to use in Hy.

Here's a Hy version:

=> (defn comp [&rest fs]
... (if (not fs) identity
...     (= 1 (len fs)) (first fs)
...     (do (setv fs (reversed fs))
...         (fn [&rest args &kwargs kwargs]
...           (for* [f fs] (setv res (apply f args kwargs))
...             (for* [f fs] (setv res (f res))))
...           res))))

gilch · 2016-12-21T23:36:51Z

(defn comp [&rest fs]
  (if (not fs) identity
      (= 1 (len fs)) (first fs)
      (do (setv fs (reversed fs))
          (fn [&rest args &kwargs kwargs]
            (for* [f fs] (setv res (apply f args kwargs))
              (for* [f fs] (setv res (f res)))) res))))

(import [time [time]])
(setv start (time))

(for [_ (range 100000)]
  ((comp inc inc inc inc) 1))

(print (- (time) start))
(print "iterative comp completed.")


(defn comp [&rest fs]
   "function composition"
   (defn compose-2 [f g]
     "compose 2 functions"
     (fn [&rest args &kwargs kwargs]
       (f (apply g args kwargs))))
   (if fs
     (reduce compose-2 fs)
     identity))

(setv start (time))

(for [_ (range 100000)]
  ((comp inc inc inc inc) 1))
(print (- (time) start))

(print "functional comp completed.")

0.6814742088317871
iterative comp completed.
1.6196417808532715
functional comp completed.

pyx · 2016-12-22T01:53:44Z

@gilch
IMHO, I do not understand the nested for loop, it works because in Hy, reversed, like in python3, returns a generator, so the first iteration on the outer loop fetch the first function, then all the work done inside inner loop, then the generator is exhausted, yielding the expected result (each function been called once and only once).

But, then this composed function can only be called once, because the function list (fs) is defined in outer scope of the function, try:

hy 0.11.0+353.gca6fd66 using CPython(default) 3.5.2 on Linux
=> (defn comp [&rest fs]
...   (if (not fs) identity
...       (= 1 (len fs)) (first fs)
...       (do (setv fs (reversed fs))
...           (fn [&rest args &kwargs kwargs]
...             (for* [f fs] (setv res (apply f args kwargs))
...               (for* [f fs] (setv res (f res)))) res))))
=> (def inc4 (comp inc inc inc inc))
=> (inc4 1)
5
=> (inc4 1)
Traceback (most recent call last):
  File "<input>", line 1, in <module>
  File "<input>", line 263, in _hy_anon_fn_1
UnboundLocalError: local variable 'res' referenced before assignment
=>

Here is my take:

(defn comp [&rest fs]
  (if (not fs) identity
      (= 1 (len fs)) (first fs)
      (do (setv rfs (reversed fs)
                first-f (next rfs)
                fs (tuple rfs))
          (fn [&rest args &kwargs kwargs]
            (setv res (apply first-f args kwargs))
            (for* [f fs]
              (setv res (f res)))
            res))))

Notice:

I used next instead of first, to make explicit the intent that we want to advance the generator once.
I used tuple call to make the rest of sequence re-usable, I guess tuple should have small footprint and better performance.
I named the first function first-f, a little bit ugly, but we need a different name or we will have the (in)famous UnboundLocalError exception, no nonlocal here, because we still support python2. Oh, lovely python scoping ... :)

Kodiologist · 2016-12-22T02:02:17Z

Yeah, the nested for loop is needlessly weird. I'd write your version, @pyx, without the extra variables, like this:

(defn comp [&rest fs]
  (if (not fs) identity
      (= 1 (len fs)) (first fs)
      (fn [&rest args &kwargs kwargs]
        (setv res (apply (get fs -1) args kwargs))
        (for [f (reversed fs)]
          (setv res (f res)))
        res)))

This has similar performance to @gilch's version.

pyx · 2016-12-22T02:59:50Z

@Kodiologist
Generating reversed list each time on-the-fly (inside the inner function) is fine, as we usually do not feed too much into comp, so the performance lost should be acceptable, but this version will call the last (of the argument list) function twice:

hy 0.11.0+353.gca6fd66 using CPython(default) 3.5.2 on Linux
=> (defn comp [&rest fs]
...   (if (not fs) identity
...       (= 1 (len fs)) (first fs)
...       (fn [&rest args &kwargs kwargs]
...         (setv res (apply (get fs -1) args kwargs))
...         (for [f (reversed fs)]
...           (setv res (f res)))
...         res)))
=> (def inc4 (comp inc inc inc (fn [n] (print "hit!") (inc n))))
=> (inc4 1)
hit!
hit!
6

Edit: spelling mistake

Kodiologist · 2016-12-22T03:03:43Z

Oh, right, whoops, I meant (cut fs -2 None -1), not (reversed fs).

pyx · 2016-12-22T03:23:44Z

I think the most common case of compose, might be that the composed fuction will be re-used a lot, think map, so on a second thought, we might better prepare the function list before entering into the scope of inner (composed) function.

And I wrote a naive time-it macro as mentioned by @gilch, I will open a new issue.

tuturto · 2016-12-22T06:48:21Z

Nice work and quite big difference in the speed already ❤️

pyx · 2016-12-22T07:18:23Z

Now that we have a proper (I hope so) timeit macro hylang/hyrule#39
And we have three implementations:

(defn comp-f [&rest fs]
   "function composition"
   (defn compose-2 [f g]
     "compose 2 functions"
     (fn [&rest args &kwargs kwargs]
       (f (apply g args kwargs))))
   (if fs
     (reduce compose-2 fs)
     identity))

(defn comp-i1 [&rest fs]
  (if (not fs) identity
      (= 1 (len fs)) (first fs)
      (fn [&rest args &kwargs kwargs]
        (setv res (apply (get fs -1) args kwargs))
        (for [f (cut fs -2 None -1)]
          (setv res (f res)))
        res)))

(defn comp-i2 [&rest fs]
  (if (not fs) identity
      (= 1 (len fs)) (first fs)
      (do (setv rfs (reversed fs)
                first-f (next rfs)
                fs (tuple rfs))
          (fn [&rest args &kwargs kwargs]
            (setv res (apply first-f args kwargs))
            (for* [f fs]
              (setv res (f res)))
            res))))

It's time to let the data speak:

=> (defn comp-f [&rest fs]
...    "function composition"
...    (defn compose-2 [f g]
...      "compose 2 functions"
...      (fn [&rest args &kwargs kwargs]
...        (f (apply g args kwargs))))
...    (if fs
...      (reduce compose-2 fs)
...      identity))

=> (defn comp-i1 [&rest fs]
...   (if (not fs) identity
...       (= 1 (len fs)) (first fs)
...       (fn [&rest args &kwargs kwargs]
...         (setv res (apply (get fs -1) args kwargs))
...         (for [f (cut fs -2 None -1)]
...           (setv res (f res)))
...         res)))

=> (defn comp-i2 [&rest fs]
...   (if (not fs) identity
...       (= 1 (len fs)) (first fs)
...       (do (setv rfs (reversed fs)
...                 first-f (next rfs)
...                 fs (tuple rfs))
...           (fn [&rest args &kwargs kwargs]
...             (setv res (apply first-f args kwargs))
...             (for* [f fs]
...               (setv res (f res)))
...             res))))

=> (print ((comp-f inc inc inc inc) 1))
5

=> (print ((comp-i1 inc inc inc inc) 1))
5

=> (print ((comp-i2 inc inc inc inc) 1))
5

=> (time-it ((comp-f inc inc inc inc) 1))
11.021359261001635

=> (time-it ((comp-i1 inc inc inc inc) 1))
8.513512686997274

=> (time-it ((comp-i2 inc inc inc inc) 1))
12.34987382899999

=> (time-it (inc4 1) (setv inc4 (comp-f inc inc inc inc)))
4.459002538002096

=> (time-it (inc4 1) (setv inc4 (comp-i1 inc inc inc inc)))
4.978545720001421

=> (time-it (inc4 1) (setv inc4 (comp-i2 inc inc inc inc)))
3.3503086150012678

Calling cut is actually slower than recurring 4 times. If we try 10 (on my machine), then it starts to gain:

=> (time-it (inc10 1) (setv inc10 (comp-f inc inc inc inc inc inc inc inc inc inc)))
11.87790252600098
=> (time-it (inc10 1) (setv inc10 (comp-i1 inc inc inc inc inc inc inc inc inc inc)))
7.043803205000586
=> (time-it (inc10 1) (setv inc10 (comp-i2 inc inc inc inc inc inc inc inc inc inc)))
5.294795802998124

pyx · 2016-12-22T12:59:15Z

Now that I think flip might not be a good idea, but may I propose other two handy functions?

pipe - the counterpart of comp

(defn pipe [&rest fs]
  "docstring goes here"
  (apply comp (reversed fs)))

in use

=> (def inc-str (pipe int inc str))
=> (inc-str "41")
'42'

juxt - I saw this one when I read about the "see also" part on clojure-docs, did not read the implementation though

(defn juxt [f &rest fs]
  "docstring goes here"
  (setv fs (cons f fs))
  (fn [&rest args &kwargs kwargs]
    (setv res [])
    (for* [f fs]
      (.append res (apply f args kwargs)))
    res))

I can imagine juxt will be super handy in data processing

=> ((juxt min max sum) (range 1 101))
[1, 100, 5050]

=> (setv [total difference product quotient] ((juxt + - * /) 24 3))
[27, 21, 72, 8.0]

=> (import [operator [itemgetter]])
=> ((juxt (itemgetter 'name) (itemgetter 'age)) {'name 'Joe 'age 42 'height 182})
['Joe', 42]

Kodiologist · 2016-12-22T19:18:21Z

@pyx, this PR is for only the three functions named in the title. Those should be in their own PR if you want them.

gilch · 2016-12-23T05:34:56Z

@pyx you could make a new issue first for those.

From the timing data we see that the cut version doesn't perform as well. It can be constructed faster, because it defers some computation to execution. This is not a good trade-off in the common case of creating a composede function once, and then calling it many times in a map over a long iterable. It's better to precompute it once. So far, I like @pyx's implementation of comp best.

You could use cut instead of reversed when precomputing. I'm not sure which is faster. We could also test for more lengths in the if, and hard code the short cases while avoiding the loop altogether. I'm not sure it would help much, but the overhead of a loop and assignment is probably more significant when there are fewer functions.

pyx · 2016-12-23T05:39:02Z

@Kodiologist, fair enough, but usually pipe is defined in terms of comp to avoid duplicate code, so that PR will be depending on this one, in this case, it might be less management effort to go with comp.

juxt, I can draft a separate PR.

pyx · 2016-12-23T05:47:52Z

@gilch Since I am new here, sometimes I don't know which way should I go, e.g, I am not sure if creating a new issue regarding yet-to-be merged PR is a proper way of giving feedback.

That's why I did not draft a PR myself in the first place, I know it should come with proper documentation and test cases, to which the requirements I am not quite sure about. I will try, for juxt, anyway.

About cut vs reversed when precomputing, I think the invocation of comp is a onetime event in most cases (data processing/analysis, web app(?), etc.), the difference is acceptable, to say the least, so whichever with better readability should win, and I vote reversed for being straightforward, cut, IMHO, is not like [::-1], which is a well known pythonic idiom.

pyx · 2016-12-23T07:44:06Z

I have juxt implemented with documentation and test, should I create a PR now, or wait until this PR get merged first then rebase, cause it will create merge conflict[1] right now, both PRs touch the *exports* part.

[1] That's why I proposed pipe and juxt into this PR instead of a separate one.

Kodiologist · 2016-12-23T19:01:42Z

@pyx Wait until this one is merged. Not because of a merge conflict, but because pipe depends on comp.

Kodiologist · 2016-12-23T19:04:39Z

@gilch I sort of regret even bringing up the subject of benchmarks because any discussion about performance is speculation outside of profiling a real program. Premature optimization is the root of all evil. Anyway, in my opinion, any of the three versions is fine for now. Do you approve this PR?

pyx · 2016-12-23T19:30:05Z

@Kodiologist , okay, I will add pipe into the mix (just juxt right now) and send a PR later. Thanks.

gilch · 2016-12-24T23:43:16Z

@gilch Since I am new here, sometimes I don't know which way should I go, e.g, I am not sure if creating a new issue regarding yet-to-be merged PR is a proper way of giving feedback.

We don't know either. Don't worry too much about it, you're doing fine.

I would approve adding juxt to Hy. Clojure has it and I use it. I'd still suggest a new PR. I feel pipe needs more discussion. That's why I don't want it added to this PR. You may open a new issue for discussing that, but I'm not inclined to approve it since Clojure doesn't have it. It mostly duplicates the behavior of the threading macros (->,->>,as->). And comp, but in reverse. It is a good point that pipe can be implemented in terms of comp, but we don't have to do it that way, and it might be more efficient if we don't. It might actually be easier to implement pipe first and then comp in terms of pipe.

Merge conflicts in this file could probably be avoided if the exports list had only one element per line. The merge conflicts in this case are trivial to resolve though.

@gilch I sort of regret even bringing up the subject of benchmarks because any discussion about performance is speculation outside of profiling a real program. Premature optimization is the root of all evil. Anyway, in my opinion, any of the three versions is fine for now. Do you approve this PR?

I normally agree with that maxim, but I feel you're using it out of context. When developing an application, it's unlikely that any given function will be on the critical path, so it's best to profile and see what's actually the bottleneck. Optimizing other parts is just a waste of time and the optimized code can be harder to maintain too.

However, when developing the standard library, we're not just making one application, but the building blocks of every application written in Hy. Therefore it's much more likely the functions we write will be on the critical path of some Hy applications, especially for something as fundamental as function composition! So I do feel it's worth some effort not to be too inefficient here. That said, proving optimality is probably an undecidable problem in the general case, so we do have to give up eventually. But you do hit diminishing returns pretty quickly in practice for a function this short.

constantly and complement are pretty trivial, and appear to be correct. I'll approve those so far. I'll also approve comp if it's changed to @pyx's version.

refi64 · 2016-12-25T00:04:29Z

I'm not really seeing a use case for compliment... what's the difference between that and (comp not f)?

Kodiologist · 2016-12-25T00:50:24Z

@kirbyfan64 Good thinking; comp not is shorter than complement, anyway.

pyx · 2016-12-25T01:51:44Z

@gilch this is my thought on pipe
When composing with, let's say, more than a handful functions, pipe is easier to work with, case study:
Assuming we are building a file processing pipeline, and have these functions:
read-file write-file parse-file rename transform-phase-1 transform-phase-2 transform-phase-3 , etc.
They are all of type

file -> file

We can build the pipeline like this:

(def process
  (pipe
    read-file
    parse-file
    transform-phase-1
    transform-phase-2
    transform-phase-3
    rename
    write-file))

It's just more natural to read in this case than comp.
If I am forced to have only one, I will go with pipe. But, there is only compose predefined (have a name) in maths, so many functional programming languages only define compose in their standard libraries, it's not because pipe is less useful, to reduce the surprise factor, so compose should stay...

If we can have both comp and pipe, I totally agree that define pipe imperatively, then define comp in terms of pipe looks better, especially in (I suppose) common scenarios like above, pipe gets used more. I might actually go as far as define both imperatively, cause these two functions' behaviours are so well known, and the logic is so trivial, that we do not expect them to be changed in the future, that means almost no maintenance burden.

pyx · 2016-12-25T01:57:15Z

@kirbyfan64 @gilch , I totally agree technically (compliment f) is just (comp not f), and in other static compiled languages, the compiler may even inline the function, but iirc, in Hy, not is not a normal function (pun not intended) but a special form, so, cannot be composed:

hy 0.11.0+357.g29b3702 using CPython(default) 3.5.2 on Linux
=> (def my-even? (comp not odd?))
Traceback (most recent call last):
  File "<input>", line 1, in <module>
NameError: name 'not' is not defined
=> not
Traceback (most recent call last):
  File "<input>", line 1, in <module>
NameError: name 'not' is not defined
=> (not True)
False

If I remember correctly, you have an issue or milestone regarding this and in, etc. But it does not work as of today.

BTW, happy holidays, everyone.

Kodiologist · 2016-12-25T01:58:45Z

If I remember correctly, you have an issue or milestone regarding this and in, etc. But it does not work as of today.

Yes, #1103. I'm guessing it will be pretty straightforward.

pyx · 2016-12-25T02:01:44Z

PS. my juxt branch is here master...pyx:juxt , just waiting for this PR, then rebase, also, if you all agree to add pipe, I can add that too.

refi64 · 2016-12-25T02:07:26Z

not is not a normal function (pun not intended)

I wonder...not should probably be shadowed just like operators like +< , right?

tuturto · 2016-12-25T07:02:55Z

I updated comp to @pyx version. Happy holidays people ❤️

Add comp, constantly and complement

a91f20d

relates hylang#1176

Fix composition order in comp

d4f4ab8

comp without parameters returns identity

7360b52

Kodiologist added 2 commits December 20, 2016 13:47

Doc edits for comp, complement, constantly

4fc5bcb

Test that (comp) returns identity exactly

529383a

Kodiologist approved these changes Dec 20, 2016

View reviewed changes

Simplify the reduce call in comp

00b121a

pyx mentioned this pull request Apr 29, 2022

time-it macro hylang/hyrule#39

Open

updated version of comp

bec04ac

gilch merged commit 71f30e8 into hylang:master Dec 25, 2016

pyx mentioned this pull request May 10, 2018

the arguments order of ap-compose is counter-intuitive #1605

Closed

Add comp, constantly and complement #1179

Add comp, constantly and complement #1179

Conversation

tuturto commented Dec 19, 2016

gilch commented Dec 19, 2016 • edited Loading

Kodiologist commented Dec 19, 2016 • edited Loading

pyx commented Dec 20, 2016 • edited Loading

tuturto commented Dec 20, 2016 • edited Loading

pyx commented Dec 20, 2016

pyx commented Dec 20, 2016 • edited Loading

tuturto commented Dec 20, 2016

gilch commented Dec 20, 2016

tuturto commented Dec 20, 2016

Kodiologist commented Dec 20, 2016 • edited Loading

tuturto commented Dec 20, 2016

pyx commented Dec 20, 2016

gilch commented Dec 21, 2016

Kodiologist commented Dec 21, 2016

gilch commented Dec 21, 2016

gilch commented Dec 21, 2016

pyx commented Dec 22, 2016

Kodiologist commented Dec 22, 2016

pyx commented Dec 22, 2016 • edited Loading

Kodiologist commented Dec 22, 2016

pyx commented Dec 22, 2016 • edited Loading

tuturto commented Dec 22, 2016

pyx commented Dec 22, 2016

pyx commented Dec 22, 2016 • edited Loading

Kodiologist commented Dec 22, 2016

gilch commented Dec 23, 2016

pyx commented Dec 23, 2016

pyx commented Dec 23, 2016 • edited Loading

pyx commented Dec 23, 2016

Kodiologist commented Dec 23, 2016 • edited Loading

Kodiologist commented Dec 23, 2016 • edited Loading

pyx commented Dec 23, 2016

gilch commented Dec 24, 2016 • edited Loading

refi64 commented Dec 25, 2016

Kodiologist commented Dec 25, 2016

pyx commented Dec 25, 2016 • edited Loading

pyx commented Dec 25, 2016 • edited Loading

Kodiologist commented Dec 25, 2016

pyx commented Dec 25, 2016

refi64 commented Dec 25, 2016

tuturto commented Dec 25, 2016

gilch commented Dec 19, 2016 •

edited

Loading

Kodiologist commented Dec 19, 2016 •

edited

Loading

pyx commented Dec 20, 2016 •

edited

Loading

tuturto commented Dec 20, 2016 •

edited

Loading

pyx commented Dec 20, 2016 •

edited

Loading

Kodiologist commented Dec 20, 2016 •

edited

Loading

pyx commented Dec 22, 2016 •

edited

Loading

pyx commented Dec 22, 2016 •

edited

Loading

pyx commented Dec 22, 2016 •

edited

Loading

pyx commented Dec 23, 2016 •

edited

Loading

Kodiologist commented Dec 23, 2016 •

edited

Loading

Kodiologist commented Dec 23, 2016 •

edited

Loading

gilch commented Dec 24, 2016 •

edited

Loading

pyx commented Dec 25, 2016 •

edited

Loading

pyx commented Dec 25, 2016 •

edited

Loading