From: Surendra Singhi
Subject: sorting behaviour - question
Date: 
Message-ID: <cof5sq$r$1@news.asu.edu>
Hi,
I have a certain function in which I am using sort to order the elements 
of the list. I know that sort is destructive, but as far as I think it 
should only destructively modify the top-level list and leave the 
elements of the list as it is.
But in my code it seems that sort is messing up the elements as well. If 
I copy the list(using copy-list) and then use sort it works fine, but if 
I directly use sort it mangles the elements of list.

Before calling the relax-temp function I am using union and 
set-difference, won't they make a copy of the list before calling 
relax-temp and hence make the call to sort safe.
The code may look bit messy, but I hope it is understandable.


(defun relax-length (props pg)
   (defun relax-temp (props &aux lev prop)
     (setf props (sort props
		      #'>  :key #'(lambda (x) (proposition-level x))))
     (setf prop (car props))
     (setf lev (proposition-level prop))
     (cond ((= lev 0) 0)
	  (t (let* ((act (find-if (lambda (x)(<= (graph-action-level x) lev))
				  (proposition-establishers prop))))
	       (1+ (relax-temp (union (set-difference props
						      (graph-action-effects act))
				      (graph-action-preconditions act))))))))
   (let ((level 0))
     (cond ((NULL props) 0)
	  (t (setq level;0)))
		   (loop for mutex-cache in (reverse (plan-graph-goal-caches pg))
			 for lev from 0 to (plan-graph-current-level pg)
			 when (present-non-mutex props mutex-cache lev)
			 return lev))))
     (if (not level)
	10000
	(relax-temp (copy-list props)))))


In particular it is the contents of (graph-action-preconditions act) 
which are getting destructively modified.

Is the problem somewhere in this function or is it in some other piece 
of code?

Thanks.

-- 
Surendra Singhi

www.public.asu.edu/~sksinghi

From: Barry Margolin
Subject: Re: sorting behaviour - question
Date: 
Message-ID: <barmar-3DAE76.15521229112004@comcast.dca.giganews.com>
In article <··········@news.asu.edu>,
 Surendra Singhi <·········@netscape.net> wrote:

> Hi,
> I have a certain function in which I am using sort to order the elements 
> of the list. I know that sort is destructive, but as far as I think it 
> should only destructively modify the top-level list and leave the 
> elements of the list as it is.
> But in my code it seems that sort is messing up the elements as well. If 
> I copy the list(using copy-list) and then use sort it works fine, but if 
> I directly use sort it mangles the elements of list.
> 
> Before calling the relax-temp function I am using union and 
> set-difference, won't they make a copy of the list before calling 
> relax-temp and hence make the call to sort safe.
> The code may look bit messy, but I hope it is understandable.

UNION and SET-DIFFERENCE may reuse parts of the original lists, so long 
as they don't modify them.  E.g.

(defvar *a* (list 1 2 3))
(defvar *b* (list 4 5 6))
(union *a* *b*) may be equivalent to

(nconc (copy-list *a*) *b*)
or
(nconc (copy-list *b*) *a*)

NCONC destructively modifies the copy of one list, but reuses the other 
list.

-- 
Barry Margolin, ······@alum.mit.edu
Arlington, MA
*** PLEASE post questions in newsgroups, not directly to me ***
From: Surendra Singhi
Subject: Re: sorting behaviour - question
Date: 
Message-ID: <cogntv$2hc$1@news.asu.edu>
Barry Margolin wrote:

> In article <··········@news.asu.edu>,
>  Surendra Singhi <·········@netscape.net> wrote:
> 
> 
>>Hi,
>>I have a certain function in which I am using sort to order the elements 
>>of the list. I know that sort is destructive, but as far as I think it 
>>should only destructively modify the top-level list and leave the 
>>elements of the list as it is.
>>But in my code it seems that sort is messing up the elements as well. If 
>>I copy the list(using copy-list) and then use sort it works fine, but if 
>>I directly use sort it mangles the elements of list.
>>
>>Before calling the relax-temp function I am using union and 
>>set-difference, won't they make a copy of the list before calling 
>>relax-temp and hence make the call to sort safe.
>>The code may look bit messy, but I hope it is understandable.
> 
> 
> UNION and SET-DIFFERENCE may reuse parts of the original lists, so long 
> as they don't modify them.  E.g.

I guess thats, what is happening, instead of creating a copy of the 
list, union and set-difference maybe using a part of the old list, and 
then when i sort it, the sort function destructively modifies the list 
and causing all the problem.
Thanks.

-- 
Surendra Singhi

www.public.asu.edu/~sksinghi
From: David Sletten
Subject: Re: sorting behaviour - question
Date: 
Message-ID: <7SJqd.24180$Uj.6373@twister.socal.rr.com>
Surendra Singhi wrote:

> 
> (defun relax-length (props pg)
>   (defun relax-temp (props &aux lev prop)
>     (setf props (sort props
>               #'>  :key #'(lambda (x) (proposition-level x))))
>     (setf prop (car props))
>     (setf lev (proposition-level prop))
>     (cond ((= lev 0) 0)
>       (t (let* ((act (find-if (lambda (x)(<= (graph-action-level x) lev))
>                   (proposition-establishers prop))))
>            (1+ (relax-temp (union (set-difference props
>                               (graph-action-effects act))
>                       (graph-action-preconditions act))))))))
>   (let ((level 0))
>     (cond ((NULL props) 0)
>       (t (setq level;0)))
>            (loop for mutex-cache in (reverse (plan-graph-goal-caches pg))
>              for lev from 0 to (plan-graph-current-level pg)
>              when (present-non-mutex props mutex-cache lev)
>              return lev))))
>     (if (not level)
>     10000
>     (relax-temp (copy-list props)))))
> 
> 

I don't see what's causing the behavior you mention, but there's 
something else I'll point out here. Why are you redefining RELAX-TEMP 
every time you call RELAX-LENGTH? Judging by your choice of names it 
appears that you intended to create a temporary function only accessible 
by RELAX-LENGTH? That's not what is happening. DEFUN creates a global 
definition. RELAX-TEMP is not only _not_ temporary, but it is also _not_ 
local either. Furthermore you are just doing unnecessary work by 
redefining it over and over. There is nothing in RELAX-TEMP that 
requires it to be defined in the scope of RELAX-LENGTH.

What you want here is FLET (or LABELS in other cases). There are two 
ways to limit access to RELAX-TEMP. One way truly does create a 
temporary function, but it gets redefined each time it is needed. The 
other creates a function that is not really _temporary_ but it is private.

The first way resembles your example:
(defun relax-length (...)
   (flet ((relax-temp (...) ...))
     ; Here we can call RELAX-TEMP
...))

Again, unless the definition of RELAX-TEMP relies on some value passed 
to RELAX-LENGTH this is wasteful to redefine repeatedly.

The alternative is to create a closure:
(flet ((relax-temp (...) ...))
   (defun relax-length (...)
      ; We can call RELAX-TEMP in here
...))

Since RELAX-LENGTH was defined within the scope of the local function 
definition of RELAX-TEMP it maintains access to it. However, outside of 
the FLET block no other code can see RELAX-TEMP.

Your nested DEFUN's look really odd to me. I can't think of any reason 
to actually do that off the top of my head.

David Sletten
From: Thomas A. Russ
Subject: Re: sorting behaviour - question
Date: 
Message-ID: <ymibrdd7qsg.fsf@sevak.isi.edu>
David Sletten <·····@slytobias.com> writes:

> 
> Surendra Singhi wrote:
> 
> > 
> > (defun relax-length (props pg)
> >   (defun relax-temp (props &aux lev prop)
> >     (setf props (sort props
> >               #'>  :key #'(lambda (x) (proposition-level x))))
> >     (setf prop (car props))
> >     (setf lev (proposition-level prop))
> >     (cond ((= lev 0) 0)
> >       (t (let* ((act (find-if (lambda (x)(<= (graph-action-level x) lev))
> >                   (proposition-establishers prop))))
> >            (1+ (relax-temp (union (set-difference props
> >                               (graph-action-effects act))
> >                       (graph-action-preconditions act))))))))
> >   (let ((level 0))
> >     (cond ((NULL props) 0)
> >       (t (setq level;0)))
> >            (loop for mutex-cache in (reverse (plan-graph-goal-caches pg))
> >              for lev from 0 to (plan-graph-current-level pg)
> >              when (present-non-mutex props mutex-cache lev)
> >              return lev))))
> >     (if (not level)
> >     10000
> >     (relax-temp (copy-list props)))))
> > 
> > 
> 
> What you want here is FLET (or LABELS in other cases). 

Actually, LABELS is needed here, since RELAX-TEMP is a recursive
function.



-- 
Thomas A. Russ,  USC/Information Sciences Institute
From: David Sletten
Subject: Re: sorting behaviour - question
Date: 
Message-ID: <zVrrd.776$Ew6.141@twister.socal.rr.com>
Thomas A. Russ wrote:


> 
> Actually, LABELS is needed here, since RELAX-TEMP is a recursive
> function.
> 
> 
> 
Oh yeah, right. I was using the _wrong_ heuristic: single function => 
FLET. But as you've pointed out, if that single function needs to 
reference itself, LABELS is required. Thanks.

David Sletten