Fork me on GitHub
Slack Archive

This page is not created by, affiliated with, or supported by Slack Technologies, Inc.

2022-08-25

Channels

#code-reviews
<
2022-08-25
>
Daniel Craig16:08:31

I wrote a function that takes a matrix and returns a vector of all the coefficients on the upper diagonals of the matrix, would love to get any feedback. Is there any way to avoid using flatten? 

;;      X_11 X_12 X_13
;; X = (X_21 X_22 X_23)
;;      X_31 X_32 X_33

;; X = [X_11 X_22 X_33 X_12 X_23 X_13]

(defn vector-from-symmetric-matrix
  "Given a symmetric matrix, represent it as a vector"
  [X]
  (let [size (count X)
        row (flatten (map range (reverse (range (inc size)))))
        col (flatten (map #(range % size) (range size)))]
    (map (partial get-in X) (partition 2 (interleave row col)))))

Noah Bogart16:08:01

if you're only flattening 1 level, you can use mapcat: 

user=> (flatten (map range (reverse (range 4))))
(0 1 2 0 1 0)
user=> (mapcat range (reverse (range 4)))
(0 1 2 0 1 0)

Daniel Craig16:08:28

oh that's great thank you!

walterl17:08:07

Also, your function will return a lazy seq (from map), and not a vector as the name implies.

Daniel Craig19:08:52

what is better, apply vector or into []

Noah Bogart19:08:15

mapv if it's the final call

walterl19:08:48

In cases like that, if mapv won't do (like if your outer fn is mapcat), wrap it in vec.

Daniel Craig19:08:16

Thanks, this is what I have now: 

(defn vector-from-symmetric-matrix
  "Given a symmetric matrix, represent it as a vector"
  [X]
  (let [size (count X)
        row (mapcat range (reverse (range (inc size))))
        col (mapcat #(range % size) (range size))]
    (mapv (partial get-in X) (partition 2 (interleave row col)))))

Noah Bogart19:08:32

that's great, no need for vec or into [] at that point

👍 1
Daniel Craig19:08:16

suppose I wanted to assert that X was square, should I use assert for that?

Daniel Craig19:08:45

I haven't seen assert used much before

walterl19:08:33

I just realized that "vector" is ambiguous in this context, so if "vector" in your fn name was meant in the mathematical sense (as it appears to be), returning a different kind of seq isn't necessarily "incorrect". Working with Clojure vectors tend to be simpler and more common, though.

walterl19:08:45

You can use assert, or add it as a :pre-condition https://clojuredocs.org/clojure.core/defn

walterl19:08:07

Or, for the Ultimate Power, you can do more thorough checks with https://clojure.org/guides/spec or https://github.com/metosin/malli

Daniel Craig19:08:24

Now I have this: 

(defn vector-from-symmetric-matrix
  "Given a symmetric matrix, represent it as a vector"
  [X]
  {:pre [(apply = (map count X))            ;; Do the rows all have the same length?
         (= (count X) (count (first X)))]}  ;; does the number of rows match the number of columns?
  (let [size (count X)
        row (mapcat range (reverse (range (inc size))))
        col (mapcat #(range % size) (range size))]
    (mapv (partial get-in X) (partition 2 (interleave row col)))))

⭐ 1