Re: [OctDev] optimized lookup

[Jaroslav Hajek]

David,
I've commited a new version that addresses the code & style issues
you've pointed out.
Also, I've noticed that Octave's current version forbids assigning to
an interval
its upper boundary, so I've corrected that, too. For your convenience,
the new version
is also attached.

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.
>
> 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.
> 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?
>
> > 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..
>
>
> 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
>
>



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

lookup.cc
Description: Text Data