Fork me on GitHub
#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:21

To support `mapcat`, beware of `flatten`: https://stuartsierra.com/2019/09/25/sequences-in-flatland

❤️ 1
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