views:

119

answers:

2

I have problems with following code: http://lisper.ru/apps/format/96

The problem is in "normalize" function, which does not work.
It fails on the fifth line: (zero-p a indexes i)

(defun normalize (a &optional indexes i)
  "Returns normalized A."
  (progn
   (format t "Data=~A ~A ~A" a indexes i)
   (if (zero-p a indexes i)
      a ;; cannot normalize empty vector
    (let* ((mmm (format t "Zero?=~a" (zero-p a indexes i)))
             (L (sqrt (+ (do-op-on * a :x a :x indexes i indexes i)
                         (do-op-on * a :y a :y indexes i indexes i)
                         (do-op-on * a :z a :z indexes i indexes i))))
             (mmm (format t "L=~a" L))
             (L (/ 1D0 L))
             (mmm (format t "L=~a" L))) ; L=1/length(A)
      (make-V3 (* (ref-of a :x indexes i) l)
                 (* (ref-of a :y indexes i) l)
                 (* (ref-of a :z indexes i) l))))))

in function "normalize" I call the macro "zero-p", which in turn calls macro "ref-of", which is the last in the chain.

(defmacro zero-p (v &optional indexes index)
  "Checks if the vector is 'almost' zero length."
  `(and (< (ref-of ,v :x ,indexes ,index) *min+*)
 (< (ref-of ,v :y ,indexes ,index) *min+*)
 (< (ref-of ,v :z ,indexes ,index) *min+*)
 (> (ref-of ,v :x ,indexes ,index) *min-*)
 (> (ref-of ,v :y ,indexes ,index) *min-*)
 (> (ref-of ,v :z ,indexes ,index) *min-*)))

Here is ref-of:

(defmacro ref-of (values coordinate &optional indexes index)
  "Please see DATA STRUCTURE for details."
  (if indexes
    (cond ((eq coordinate :x) `(aref ,values (aref ,indexes ,index)))
   ((eq coordinate :y) `(aref ,values (+ 1 (aref ,indexes ,index))))
   ((eq coordinate :z) `(aref ,values (+ 2 (aref ,indexes ,index))))
   (T (error "The symbol ~S is not :X, :Y or :Z." coordinate)))
    (cond ((eq coordinate :x) `(aref ,values 0))
 ((eq coordinate :y) `(aref ,values 1))
     ((eq coordinate :z) `(aref ,values 2))
     (T (error "The symbol ~S is not :X, :Y or :Z." coordinate)))))

Also, in "normalize" I call the macro "do-op-on", which calls "ref-of" as well.

(defmacro do-op-on (op name1 coord1 name2 coord2 &optional is1 i1 is2 i2)
  "Example: (do-op-on * A :x B :y i n) == A[i[n]].x*B.y"
  `(,op (ref-of ,name1 ,coord1 ,is1 ,i1) (ref-of ,name2 ,coord2 ,is2 ,i2)))

As a result, instead of having this: (aref some-array 0) I have (aref NIL NIL) which is created in "ref-of".

I suppose that I lose the symbol A from the call (normalize A). I just feel that the symbol does not survive the macroexpanson. The thing is, macroexpansoin works in REPL for each macro independently.

Can anyone explain where is the mistake?

+7  A: 

Note that the macros ZERO-P and REF-OF are expanded when you evaluate, compile or load the DEFUN for NORMALIZE. Their arguments are the symbols INDEXES and INDEX, and both of those are not NIL, so the IFs in the REF-OF forms will all take the first branch and expand into AREF forms where INDICES and INDEX can't be bound to NIL. In short, you've gotten evaluation time and macroexpansion time confused, which is an easy thing to do when you are just starting with macros.

However, when you call the function NORMALIZE with only one argument, the variables INDICES and INDEX are bound to a default value of NIL, so AREF complains that it's getting invalid arguments.

The best solution I can offer you is to just make ZERO-P and REF-OF into functions instead of macros. They'll work just fine as functions, and you shouldn't make something a macro unless you're sure it needs to be a macro. If you really do want to keep them as macros, give default values to INDICES and INDEX that make sense and get rid of the optional values in REF-OF and ZERO-P---I'm pretty sure having INDEX default to 0 and INDICES default to #(0 1 2) will work.

EDIT to add: Wanting to avoid function call overhead is almost certainly not a good reason to use macros in Lisp. In the first place, you shouldn't even worry about function call overhead until after you've done some profiling and testing. In the second place, if you do have a problem with function call overhead, you should DECLARE the functions in question INLINE instead of using macros to do the inlining.

EDITed again to add: If your functions are being expanded inline, your compiler should be able to figure out that it can replace

(cond ((eq :x :x) 'foo)
      ((eq :x :y) 'bar)
      ((eq :x :z) 'quux)
      (t (error "~A is not one of :X, :Y or :Z" :x))

with

'foo
Pillsy
PRAGMA was a misprint at the last moment. PROGN.
avp
Hey, thanks for the answer. I just want to clarify one thing.You've said: "...shouldn't make something a macro unless you're sure...". So how can I be sure? I introduced macros to simplify the code, not to add any functions to the library... Is this reason good enough?
avp
No, if you just want to simplify your code, always see if you can factor things into smaller functions first. Functions are simpler than macros---they're easier to test, they can be traced, and they show up in debugger backtraces. They can also be used directly with functions like `MAPCAR`, `REDUCE` or `EVERY`.
Pillsy
Thanks! You helped me very much.
avp
+1  A: 

What is PRAGMA? It is not standard. Maybe you mean PROGN (which is not even necessary since DEFUN supplies an implicit PROGN)?

Why all the macros? Is there some reason to disallow a form like (reduce (lambda (r c) (* (ref-of A c) r)) (list :x :y :z) :initial-value 1)? This seems like a case of premature optimization.

Pillsy's answer is right on: when REF-OF is expanded (from the usage of ZERO-P), its INDEXES will have the symbol INDEXES as its value, not the value passed into NORMALIZE (like wise INDEX will be I).

Chris Johnsen
You are right, I confused the word. I added it at the last moment for debugging. Fixed.
avp
About macros: how can I be sure that the code would be executed in compile-time otherwise?
avp
Yes, '(reduce (lambda (c) (ref-of A c)) (list :x :y :z) :initial-value 1)' is good idea, thanks.
avp