Re: [help-GIFT] gift-write-feature-descs segfaults... (is the GIFT broken?)

[David Squire]

Jonas Lindqvist wrote:
> > > This is undoubtedly my fault, for not checking the other binaries.
>
> I wouldn't call this anyone's "fault". I expected the compiler to 
> help, and was surprised that it didn't.
>
> > There is a general lesson here. Before committing any patches, a full 
> > build of the whole system, and test cases run (e.g. adding a 
> > collection and testing that it can be queried).
>
> Why not instead see if we can get the compiler/linker to help out 
> finding missing functions?
> Adding a collecting and then querying it, will test just the simple 
> adding and querying in the test. Nothing more.
> It's difficult to test all different paths of execution possible, 
> although testing some paths is better than nesting none.
> (Would an indexing-run followed by some queries assert that all the 
> different libs work as intended?)
I was not intending to suggest run-time tests as a replacement for 
compile-time error catches. But I still think that a full build and test 
should be done before a commit... of course we should have a "make test" 
for this.

It is always hard (even impossible) to guarantee that all execution 
paths are exercised, but we do have a pretty good idea of the main ones:

add a collection
get a random set of images from that collection
query the collection for images similar to some query image

> For example, using the latest CVS version of the GIFT, I was able to 
> run gift-add-collection on a collection, and query it for a random set 
> of images. No problem. However, whenever asking for any similar 
> images, it failed.

Well of course I meant an actual similarity query. From my point of view 
getting random images is not really a query (though that is how it 
appears in MRML and the code). Originally (before MRML) I did this in 
the interface, not by querying.

> I'm sure some other modification could break a not-so-often used 
> functions while still appearing to work when running some basic 
> "normal" tests.
> By using the compiler/linker to alert us, we would be able to find 
> these errors in compile time instead of in runtime.
> Much nicer in my opinion.


> One other tool would be grep. If I decide to alter the signature of a 
> function/method, then grep-ing for that name in the source-files, 
> could lead me to places where that function is used.
> Only... this is not convenient in this case since the GIFT source code 
> is so full of unused junk that isn't compiled anyway. (Is 
> gabor981029.c an example of primitive source control?) I get too many 
> matches in files that aren't needed.

Yep. It's ancient history. The feature extraction stuff was not added to 
GIFT by me (though it's my fault it didn't get cleaned up before being 
added). So, the FeatureExtraction directory was originally just a copy 
of a directory of mine that included all sorts of stuff - old versions, 
test code, functions written for general use with RGB/HSV, etc. There's 
no reason not to clean it up.

My current feeling is that the currently evolving optimized version of 
"GIFT features 1.0" should stay in the trunk, and if I (or others) get 
around to writing other feature extractors that do different things, 
they should be available not as replacements but as alternatives, 
perhaps selected via an optional argument to gift-extract-features.pl

> > > Yes, normally C screams and yells at you, but the gift was written
> > > without function prototypes, so the compiler cant tell, until link 
> > > time.
>
> Well, it didn't even complain when linking...
> -Wmissing-prototypes could be used to at least get a compiler warning, 
> but I'm not sure why the linker didn't speak up.

Perhaps dynamic linking? Just guessing here.

> > So, at some point a decision was made to do without the prototypes. 
> > In my original version (from Viper, before it was bundled with the 
> > GIFT), that starts:
> > #include "extract_features.proto"
> >
> > I would guess that that changed because someone did not have access 
> > to cproto...
>  ...include all necessary header-files, the old-fashioned way...
> Maybe we should fix that now? Or is there a reason not to?
Not that I can think of.


Regards,

David


--
Dr David McG. Squire, Senior Lecturer, on sabbatical in 2006
Caulfield School of Information Technology, Monash University, Australia
CRICOS Provider No. 00008C       http://www.csse.monash.edu.au/~davids/