Fork me on GitHub
#specter
<
2017-11-03
>
borkdude15:11:13

So, this works:

(def data ;; only x and y are allowed
  [{:name "x" :rels ["x" "y"]} ;=> fine, keep as is
   {:name "y" :rels ["x" "y" "z"]} ;=> keep only allowed rels: {:name "y" :rels ["x" "y"]}
   {:name "y" :rels ["z"]} ;=> no allowed rels, remove map completely
   {:name "z" :rels ["x" "y"]}] ;=> disallowed name, remove completely
  )

(def disallowed? (complement #{"x" "y"}))

(setval
 [ALL
  (multi-path
   [:rels ALL
    (pred disallowed?)]
   (selected?
    (multi-path
     [:rels empty?]
     [:name (pred disallowed?)])))]
 NONE
 data)
I was wondering if it’s possible to first express [:name (pred disallowed?)], since it might be cheaper to look at the name first. Then it doesn’t have to process the :rels. E.g.
(setval
 [ALL
  (multi-path
   (selected? :name disallowed?)
   [:rels ALL disallowed?]
   (selected? :rels empty?))]
 NONE
 data)
But this triggers a ClassCastException probably because NONE is treated as a map or something.

borkdude15:11:44

When I replace NONE with {} it kind of works. Maybe Specter paths should be able to play nicely with NONE?

borkdude15:11:40

This works:

(setval
 [ALL
  (multi-path
   (selected? :name disallowed?)
   [#(not= NONE %) :rels ALL disallowed?]
   (selected? #(not= NONE %) :rels empty?))]
 NONE
 data)

borkdude15:11:57

(defn SOME? [x]
  (not= NONE x))

(setval
 [ALL
  (multi-path
   (selected? :name disallowed?)
   [(pred SOME?) :rels ALL disallowed?]
   (selected? (pred SOME?) :rels empty?))]
 NONE
 data)

borkdude15:11:57

I quite like the last solution. It almost reads literally like my requirement: - drop when the name is disallowed - drop all :rels that are disallowed - drop when no :rels are left

nathanmarz15:11:44

when you remove an element from a sequence, then future navigation in the same path will start at NONE

nathanmarz15:11:16

that's the correct behavior from Specter, and getting a ClassCastException is expected and desired

nathanmarz15:11:43

that solution with SOME? is fine

nathanmarz15:11:52

you don't need the pred around SOME?

nathanmarz15:11:58

but you should have it around disallowed?

nathanmarz15:11:08

or define disallowed? as (pred (complement ...))

borkdude15:11:31

I’m not sure why it’s desired? Navigating to NONE could semantically be treated as a dead end?

nathanmarz15:11:48

actually nvm, you don't need it around disallowed? since it's defined globally

nathanmarz15:11:35

the element in the sequence was removed, so it's navigated to the "void", represented consistently in Specter as NONE

borkdude15:11:44

well, in my ‘real’ code, disallowed is not able to be defined globally, but that’s why I wrapped it in a pred in the let. Now I don’t understand why I don’t need the pred around SOME?, because it’s a global?

nathanmarz15:11:01

multi-path always executes every subpath in order

nathanmarz15:11:09

SOME? is statically known to be a function

nathanmarz15:11:20

so specter can figure that out at "compile-time"

borkdude15:11:23

ok, so Specter intentionally allows navigating to NONE, so the user can do something with this?

nathanmarz15:11:43

AFTER-ELEM and BEFORE-ELEM navigate to NONE

borkdude15:11:09

cool! so maybe Specter could use a primitive like SOME?

borkdude15:11:27

I’m fine with my own for now

nathanmarz15:11:57

needing that function is very esoteric

nathanmarz15:11:25

btw a better way to implement it is (defn SOME? [v] (not (identical? NONE v)))

borkdude15:11:10

because eventually = will dispatch to that anyway

nathanmarz15:11:22

yea, avoids some overhead

borkdude15:11:02

cool, I learned something 🙂

borkdude16:11:10

Hack which makes SOME? obsolete:

(alter-var-root #'com.rpl.specter.impl/NONE (fn [_] {}))

(setval
 [ALL
  (multi-path
   (selected? :name disallowed?)
   [:rels ALL disallowed?]
   (selected? :rels empty?))]
 NONE
 data)

borkdude16:11:12

AFTER-ELEM and BEFORE-ELEM still work

borkdude16:11:31

hmm, but this is going to break other stuff probably, because it seems that Clojure uses a singleton object for representing the empty hash-map

borkdude16:11:03

(identical? (hash-map) (hash-map)) ;;=> true

nathanmarz16:11:27

becomes very hard to set values to empty maps

borkdude16:11:12

This could work though:

(def my-NONE {::NONE true})
(alter-var-root #'com.rpl.specter.impl/NONE (fn [_] my-NONE))

(defn SOME? [x]
  (not (identical? NONE x)))

;; no SOME? needed

(setval
 [ALL
  (multi-path
   (selected? :name disallowed?)
   [:rels ALL disallowed?]
   (selected? :rels empty?))]
 NONE
 data)

(setval AFTER-ELEM 3 [1 2])
(setval BEFORE-ELEM 3 [1 2])
since (identical? {::NONE true} {::NONE true}) ;;=> false

borkdude16:11:04

Don’t know if there is anything against changing this implementation detail?

borkdude16:11:12

Could make life with NONE somewhat easier

nathanmarz16:11:36

no, it's better to error when trying to navigate on NONE

nathanmarz16:11:20

this code makes NONE magically "work" for map navigators

nathanmarz16:11:31

but it will still fail for all the other navigators

nathanmarz16:11:50

better to handle it explicitly in code like this

borkdude16:11:49

Looking at the vanilla Clojure (which I didn’t write prior to writing Specter), it’s comparable length wise: https://gist.github.com/borkdude/5f9a4ae710217e893a9462ff90b6cac3#file-specter-clj-L20

nathanmarz16:11:38

it changes all the vectors to seqs

borkdude16:11:56

true. in this case I didn’t need the vectors much longer as it went out as JSON anyway

nathanmarz16:11:30

length isn't always the best metric of code quality

nathanmarz16:11:56

besides maintaining types, I find the Specter version far more readable

nathanmarz16:11:14

pretty much read left to right instead of having lots of nested expressions

borkdude17:11:51

The Clojure version appears to be twice as fast (running 100k times with time, keep wrapped in doall, I know, not a robust profiling tool, but still?)

borkdude17:11:40

Around 200ms for Clojure and 400ms for Specter

nathanmarz17:11:50

there's probably a bunch of micro-optimizations that can be done

nathanmarz17:11:10

empty? is slow on vectors

borkdude17:11:40

Could try seq

nathanmarz17:11:41

#(= 0 (.length ^IPersistentVector v)) is fastest way I've found

borkdude17:11:55

oh no wait, that’s the opposite 😛

nathanmarz17:11:42

can also change to:

(setval
 [ALL
  (if-path (selected? :name disallowed?)
   STAY
   (multi-path
    [:rels ALL disallowed?]
    (selected? :rels empty?)))]
 NONE
 data)

nathanmarz17:11:53

so it stops doing work after removing on first condition

nathanmarz17:11:43

#(-> % :name disallowed?) might be faster

nathanmarz17:11:04

same with #(-> % :rels empty?)

borkdude17:11:46

(def my-empty? (fn [v] (= 0 (.length ^clojure.lang.IPersistentVector v))))

(time
 (dotimes [i 100000]
   (setval
    [ALL
     (if-path #(-> % :name disallowed?)
              STAY
              (multi-path
               [:rels ALL disallowed?]
               #(-> % :rels my-empty?)))]
    NONE
    data)))
Shaves off roughly 80ms

nathanmarz17:11:44

since specter version maintains types and clojure version constructs a different type, it's hard to say how much of difference comes from that

nathanmarz18:11:32

@borkdude the inner call to remove is also not realizing the full computation

nathanmarz18:11:56

should wrap that in doall to make comparison fair

borkdude18:11:27

@nathanmarz yes, did that, yields about 20ms extra

tanzoniteblack18:11:39

(defn vanilla []
  (doall (keep
          (fn [m]
            (if-not (disallowed? (:name m))
              (if-let [new-rels (doall (seq (remove disallowed? (:rels m))))]
                (assoc m :rels new-rels))))
          data)))
making this non-lazy (and measured with criterium)
Evaluation count : 558918 in 6 samples of 93153 calls.
             Execution time mean : 1.115634 µs
    Execution time std-deviation : 37.921802 ns
   Execution time lower quantile : 1.082254 µs ( 2.5%)
   Execution time upper quantile : 1.172033 µs (97.5%)
                   Overhead used : 1.603741 ns
This specter version:
(defn specter-v2 []
  (setval [ALL
           (if-path #(-> % :name disallowed?)
                    STAY
                    (multi-path [:rels ALL disallowed?]
                                #(-> % :rels empty?)))]
          NONE
          data))
also measured with criterium:
Evaluation count : 344118 in 6 samples of 57353 calls.
             Execution time mean : 1.732753 µs
    Execution time std-deviation : 12.525124 ns
   Execution time lower quantile : 1.716643 µs ( 2.5%)
   Execution time upper quantile : 1.744641 µs (97.5%)
                   Overhead used : 1.603741 ns

tanzoniteblack18:11:21

so yeah...still faster with the hand-written clojure, but no where near as much as it was when factoring in the fact that one was lazy and the other not before

tanzoniteblack18:11:11

so...meh. Unless performance is absolutely critical to you, then it's just a style preference here? And if performance is absolutely critical...then always measure very carefully whatever you're doing

borkdude21:11:19

@tanzoniteblack That criterium benchmark doesn’t draw a very different picture than what I already had

borkdude21:11:25

I had roughly 350ms for specter, 220 for clojure (x 1.5), criterium: x 1.55

borkdude21:11:41

but thanks for measuring

tanzoniteblack21:11:56

🙂 wasn't saying my numbers were different then yours, I just couldn't find hard numbers for your final versions to directly compare, so decided to play with it myself

tanzoniteblack21:11:57

also, I didn't see much of a difference between your custom empty and directly using empty? when measured with criterium. Don't remember the exact numbers, but only like 50-100 nanoseconds difference with the different empties swapped? Something within the realm of measuring error with the final result being in microseconds, anyways

tanzoniteblack21:11:04

(just as an FYI)

borkdude22:11:19

which is also slower than the non-transducer version