Hi,
I am trying to learn iterate, and ran into difficulties.
I am rewriting the following code converts a sparse matrix into CSC
format (used by Matlab, UMFpack, etc):
(defun matrixp (array)
(and (arrayp array) (= (array-rank array) 2)))
(defun pack-csc (matrix &optional (zero-test #'zerop))
(assert (matrixp matrix))
(let ((ap nil)
(index 0)
(ai nil)
(ax nil)
(number-of-rows (array-dimension matrix 0)))
(dotimes (column (array-dimension matrix 1))
(push index ap) ; start of column
(dotimes (row number-of-rows)
(let ((element (aref matrix row column)))
(unless (funcall zero-test element)
(push element ax)
(push row ai)
(incf index)))))
(push index ap)
(values (nreverse ap) (nreverse ai) (nreverse ax))))
(defparameter *m* (make-array '(5 5) :initial-contents
'((2 3 0 0 0)
(3 0 4 0 6)
(0 -1 -3 2 0)
(0 0 1 0 0)
(0 4 2 0 1))))
(pack-csc *m*)
I want to rewrite pack-csc using iter:
(defun pack-csc2 (matrix &optional (zero-test #'zerop))
(assert (matrixp matrix))
(iter
(with number-of-rows = (array-dimension matrix 0))
(with index = 0)
(for column from 1 to (array-dimension matrix 1))
(collect index into ap)
(iter
(for row from 1 to number-of-rows)
(let ((element (aref matrix row column)))
(unless (funcall zero-test element)
(collect element into ax)
(collect row into ai)
(incf index))))
(finally
(collect index into ap)
(values ap ai ax))))
but get the following warnings (in sbcl):
; caught WARNING:
; undefined variable: AI
;
; caught WARNING:
; undefined variable: AX
; (ITERATE:COLLECT CL-SPARSEMATRIX::INDEX CL-SPARSEMATRIX::INTO
; CL-SPARSEMATRIX::AP)
;
; caught STYLE-WARNING:
; undefined function: COLLECT
;
; caught WARNING:
; undefined variable: INTO
;
; caught WARNING:
; These variables are undefined:
; AI AX INTO
;
; caught STYLE-WARNING:
; This function is undefined:
; COLLECT
;
; compilation unit finished
; caught 4 WARNING conditions
; caught 2 STYLE-WARNING conditions
I the problem is with the collects that are inside unless and finally.
I seem to be abusing iter somehow, but don't know what the problem is.
Improving the elegance of the code above would also be appreciated.
Thanks,
Tamas
On Jun 28, 8:27 am, Tamas Papp <······@gmail.com> wrote:
[...]
> (defun pack-csc2 (matrix &optional (zero-test #'zerop))
> (assert (matrixp matrix))
> (iter
> (with number-of-rows = (array-dimension matrix 0))
> (with index = 0)
> (for column from 1 to (array-dimension matrix 1))
> (collect index into ap)
> (iter
> (for row from 1 to number-of-rows)
> (let ((element (aref matrix row column)))
> (unless (funcall zero-test element)
> (collect element into ax)
> (collect row into ai)
> (incf index))))
> (finally
> (collect index into ap)
> (values ap ai ax))))
> I the problem is with the collects that are inside unless and finally.
> I seem to be abusing iter somehow, but don't know what the problem is.
> Improving the elegance of the code above would also be appreciated.
You're essentially correct about the FINALLY clause: it executes the
code after the iteration has ended normally, as the ITERATE manual
notes. If you macroexpand the ITERATE form, you'll notice that the
COLLECT form in the FINALLY clause isn't turned into anything. The
other major problem you're having is that the variables AI and AP only
exist within the innermost ITERATE form, so they're gone by the time
you're done and returning stuff, and even if they weren't, they'd only
hold values from the last trip through the outer loop.
Fortunately, ITERATE provides you with relatively easy ways to
sidestep all these problems.
Here's a correct implementation that shows how to sidestep these
problems. HTH:
(defun pack-csc2 (matrix &optional (zero-test #'zerop))
(assert (matrixp matrix))
(iter outer ; You can name ITER blocks so you can return to them
; later. I called this one OUTER.
(with number-of-rows := (array-dimension matrix 0))
(with index := 0)
;; If you stuff the starting value of INDEX into AP at the
;; beginning, you can stuff subsequent values in when they're
;; available and have them all in the right place.
(when (first-time-p)
(collect index :into ap))
;; In Common Lisp, array indices start from 0, like in C, rather
;; than from 1, like in Fortran. The ITERATEish way of reflecting
;; this is to use (FOR x :FROM 0 :BELOW n), which terminates
;; unless X <= N.
(for column :from 0 :below (array-dimension matrix 1))
(iter
(for row :from 0 :below number-of-rows)
;; If you're going to bind a variable to a new value on every
;; trip through the loop, use the (FOR x := ...) clause instead
;; of a LET form.
(for element := (aref matrix row column))
(unless (funcall zero-test element)
;; Because I named the outer loop OUTER, I can refer to
;; variables in it from the inner loop using the IN clause.
(in outer
(collect element :into ax)
(collect row :into ai))
(incf index)))
;; Now I just collect INDEX into AP normally.
(collect index :into ap)
(finally
(return-from outer (values ap ai ax)))))
I claim it's correct because it returns the same values as PACK-CSC
for your test case:
> (mapcar #'equal
(multiple-value-list (pack-csc *m*))
(multiple-value-list (pack-csc/iter *m*)))
==> (T T T)
As for improving the elegance of your code, you might want to
investigate the Common Lisp functions ROW-MAJOR-AREF and ARRAY-ROW-
MAJOR-INDEX. They can be very useful in applications where you have to
iterate over the elements of multi-dimensional arrays.
Cheers,
Pillsy
Pillsy <·········@gmail.com> writes:
> (defun pack-csc2 (matrix &optional (zero-test #'zerop))
> (assert (matrixp matrix))
> (iter outer ; You can name ITER blocks so you can return to them
> ; later. I called this one OUTER.
> (with number-of-rows := (array-dimension matrix 0))
> (with index := 0)
> ;; If you stuff the starting value of INDEX into AP at the
> ;; beginning, you can stuff subsequent values in when they're
> ;; available and have them all in the right place.
> (when (first-time-p)
> (collect index :into ap))
> ;; In Common Lisp, array indices start from 0, like in C, rather
> ;; than from 1, like in Fortran. The ITERATEish way of reflecting
> ;; this is to use (FOR x :FROM 0 :BELOW n), which terminates
> ;; unless X <= N.
> (for column :from 0 :below (array-dimension matrix 1))
> (iter
> (for row :from 0 :below number-of-rows)
> ;; If you're going to bind a variable to a new value on every
> ;; trip through the loop, use the (FOR x := ...) clause instead
> ;; of a LET form.
> (for element := (aref matrix row column))
> (unless (funcall zero-test element)
> ;; Because I named the outer loop OUTER, I can refer to
> ;; variables in it from the inner loop using the IN clause.
> (in outer
> (collect element :into ax)
> (collect row :into ai))
> (incf index)))
> ;; Now I just collect INDEX into AP normally.
> (collect index :into ap)
> (finally
> (return-from outer (values ap ai ax)))))
Thank you, you have helped a lot.
Tamas