Re: [OctDev] optimized lookup

[Jaroslav Hajek]

On Feb 18, 2008 2:25 PM, David Bateman <address@hidden> wrote:
> Jaroslav Hajek wrote:
> > I agree. My intention was to put this into Octave-Forge for anyone 
> > interested,
> > so that anyone installing the Octave-Forge package gets the optimized
> > version (I think .oct files have "higher priority" than .m files).
> > If anyone of you Octave's maintainers decides that the function is worth
> > including into Octave itself, you can simply do it (and possibly remove
> > it from the Octave-Forge package).
> >
> Looking at the speed improvements on the basis of the speed it makes
> sense to include this code. However, speed is not the only basis to
> consider. The next point is how often will the code in fact be used.
> "lookup" is used by all of the interpolation functions for the "nearest"
> and "linear" interpolation functions. Therefore, it is likely that it
> will be used often. The next point to consider is the maintainability of
> the code. The code is relatively short, so ok on that point as well.
> However, it doesn't follow the coding style of the rest of Octave at
> this point.

Yes. After all, it is not part of Octave at this moment. If you want
to include it,
I'll certainly adjust the style to meet Octave's standards.

>
> 1) The copyright should be in /* .... */ comments
> 2) The Author field shoudl be below the copyright section

> 3) ret_idx_type should use octave_idx_type instead that is always the
> indexing (32- or 64-bit) that is used
> 4) ret_idx_array type should in fact be an NDArray for consistency with
> the original lookup function and so other Octave functions. Yes this
> limits the indexing in a single dimension to 2^52 elements, but is that
> really an issue.

Yes, I've noticed this problem - that's why the typedefs are isolated on the
beginning.
I was just wondering, now that Octave has integer types, whether or not
real-valued return indices are a relic of the past. Such a change should
probably go unnoticed in most places with several exceptions like assert.
Returning index arrays as integers is certainly more sound; OTOH, there's
the backward and Matlab compatibility issue.

> 5) The indentation style in Octave is more like
>
> if (a == b)
>   {
>     /* Code here */
>   }
> else
>   {
>     ...
>    }
>
> rather than
>
> if (a == b) {
>   /* Code here */
> } else {
>   ...
> }
>
> that you've used. There are other coding style issues as well, but that
> is the one that stuck out. If you can fix at least the ret_idx_type and
> ret_idx_array issues, and John accepts the code, the other issues will
> be easy enough to fix when the code is committed. John do you want this
> function in the 3.1.x tree?
>

Again, no problem adjusting that.

> > Perhaps it would be a better idea to create a whole new package that
> > would contain
> > optimized (or otherwise improved) versions of Octave's library
> > functions, so that they can
> > be tested and eventually included.
> >
> Only if we can't resolve rapidly whether this function should be
> included in the 3.1.x tree of Octave or not.. In any case if it is
> included in the 3.1.x tree, I believe we should keep octave-forge
> targeted to Octave 3.0.x for the moment, so it might make even more
> sense now that I think about it to have lookup.cc in octave-forge if it
> is accepted for Octave 3.1.x..
>
>

That was precisely my intention; I sent the function directly to
octave-maintainers
at first, but received no response, presumably because there was a lot
of more important
stuff going on concerning the 3.0.1 release. The Octave-Forge "main"
repo claims to
collect "Packages which may eventually be included in the main octave
distribution."
Several packages in main/ mirror Octave's library structure, like
main/general and main/linear-algebra; that's why I guessed it is a
good place to put my code into.

> D.
>
>
>
>
> >
> >
>
>
> --
> David Bateman                                address@hidden
> Motorola Labs - Paris                        +33 1 69 35 48 04 (Ph)
> Parc Les Algorithmes, Commune de St Aubin    +33 6 72 01 06 33 (Mob)
> 91193 Gif-Sur-Yvette FRANCE                  +33 1 69 35 77 01 (Fax)
>
> The information contained in this communication has been classified as:
>
> [x] General Business Information
> [ ] Motorola Internal Use Only
> [ ] Motorola Confidential Proprietary
>
>

best,

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