Re: Efficiency in 13980:bd2be36fd949

[Ben Abbott]

On Dec 3, 2011, at 2:39 PM, Ben Abbott wrote:

> On Dec 3, 2011, at 1:45 PM, John W. Eaton wrote:
> 
>> On  3-Dec-2011, Jordi Gutiérrez Hermoso wrote:
>> 
>> | I noticed that this change introduces a bit of a needless slowdown:
>> | 
>> |     http://hg.savannah.gnu.org/hgweb/octave/rev/bd2be36fd949
>> | 
>> | Can you please rephrase the condition so that warn is at the front and
>> | the short-circuiting avoids the call to unique?
>> | 
>> | Sorry I don't do it myself. I don't currently have access to my dev 
>> machine.
>> 
>> I could do that, but why should this warning only be issued once per
>> Octave session?  Shouldn't it be checked for each image?  Given all
>> the other things that image has to do, does it really represent a large
>> slowdown if we made the check on each call?
>> 
>> Also, the test is somewhat simplistic.  For example, if I understand
>> correctly what it is intended to do, it fails for something like
>> 
>> x = y = (1:100)/5; image (x, y, rand (100, 100, 3))
>> 
>> jwe
> 
> ouch !
> 
> I'm embarrassed to say I didn't consider numerical round off problems :-(
> 
> Not sure what the best way to resolve that is ... but when I do ...
> 
> N = 100;
> x = y = (1:N)/5;
> 
> tic
> for n = 1:1000
>  dx = diff (x);
>  dy = diff (y);
>  dx = std (dx) / mean (abs (dx));
>  dy = std (dy) / mean (abs (dy));
>  tol = 100*eps;
>  if (any (dx > tol) || any (dy > tol))
>    warning ("Image does not map to non-linearly spaced coordinates")
>  endif
> endfor
> toc
> 
> ... I get ....
> 
> Elapsed time is 1.3 seconds.
> 
> 1.3 msecs looks acceptable to me.
> 
> If this looks ok, I can push the attached.
> 
> Ben
> 
> <changeset.patch>

I pushed the change.

http://hg.savannah.gnu.org/hgweb/octave/rev/e58963b5c0b1

Ben