Re: purpose of 10486:4e64fbbd5c58

[Jaroslav Hajek]

On Thu, May 6, 2010 at 8:14 AM, Jaroslav Hajek <address@hidden> wrote:
> On Wed, May 5, 2010 at 8:28 PM, John W. Eaton <address@hidden> wrote:
>> On 30-Apr-2010, Jaroslav Hajek wrote:
>>
>> | The reason is that if isindex (I) is true, I expect isindex ([1; I])
>> | to be also true, expect it to be usable in sub2ind etc. If there *has*
>> | to be an iconsistency like this, I'd prefer it to be as non-intrusive
>> | as possible.
>>
>> OK.  Since I'm adding this (mis)feature only to allow people to run
>> existing Matlab code with Octave and Matlab doesn't seem to have
>> isindex, I don't expect this to cause trouble for them.
>>
>> | I understand your motivation for this, though I still consider this to
>> | be the same kind of misfeature like the short-circuiting & and |
>> | operators. As I said, I've exhausted my arguments. The decision is up
>> | to you.
>>
>> I checked in the following change:
>>
>>  http://hg.savannah.gnu.org/hgweb/octave/rev/1834132fb50b
>>
>> It's the same as the last patch I proposed, except it avoids using
>> feval to call Fwarning to set the warning state so overloading can't
>> cause trouble.
>>
>> The (stupidly inconsistent, I agree) behavior is only enabled if the
>> Octave:allow-noninteger-ranges-as-indices warning state is set to "on"
>> or "off".  If Octave is started with --traditional, the default value
>> is "on".  Otherwise, the default is "error".
>>
>> jwe
>>
>
> Now I'm getting this:
>
> octave:1> (1:10)(1.1:10.1)
> error: rounding non-integer range used as index to nearest integer
>
> This error message is really confusing.
>
> Further, the following no longer works:
>
> a = rand (1, 10);
> idx = 1.1:10.1;
>
> try
>  b = a(idx);
> catch
>  [err, id] = lasterr ();
>  if (any (strcmp (id, {"Octave:invalid-index", 
> "Octave:index-out-of-bounds"})))
>    disp ("indexing error occured");
>  else
>    rethrow (lasterror);
>  endif
> end_try_catch
>
> I think my original solution was better in that it left the normal
> behavior intact. If you really insist on the warning ID solution, I
> think the message should be just "non-integer range used as index".
>
> I propose to revert 10605:1834132fb50b and apply the patch I sent
> earlier (after completing it).
>
> regards
>

Besides changing the error ID, another downside of this change is that
isindex gets up to 30x slower with simple expressions, probably due to
the fact that Fwarning is slow:

old:

address@hidden:~/devel/octave/main> octave -q
octave:1> tic; arrayfun (@isindex, 1:1e5); toc
Elapsed time is 0.065716 seconds.

new:

address@hidden:~/devel/octave/main> ./run-octave -q
octave:1> tic; arrayfun (@isindex, 1:1e5); toc
Elapsed time is 1.6982 seconds.

A disappointment, considering that I designed isindex to be an efficient query.

Please, please, please, can I have just a simple flag to turn off this
whole ugliness? It can be on by default, I don't care, I'll just
disable it in my settings. So far I never needed to compile Octave
with extra patches, I'd like to avoid that.

thank you.

-- 
RNDr. Jaroslav Hajek, PhD
computing expert & GNU Octave developer
Aeronautical Research and Test Institute (VZLU)
Prague, Czech Republic
url: www.highegg.matfyz.cz