amplab / keystone

Simplifying robust end-to-end machine learning on Apache Spark.
http://keystone-ml.org/
Apache License 2.0
470 stars 117 forks source link

Fixing daisy feature ordering to match SIFT. Closes #186. #205

Closed etrain closed 8 years ago

etrain commented 8 years ago

This fix transposes the order of features coming out of the Daisy Extractor so that it's a drop in replacement for the SIFT featurizer -- logically these things do different things but at least "Image Descriptors" should have a common structure. Closes #186

shivaram commented 8 years ago

LGTM

shivaram commented 8 years ago

Would be good to have a test comparing them somehow ? i.e. enforcing they are the same in Daisy and SIFT ?

etrain commented 8 years ago

I can do that, but now I'm wondering if an ImageDescriptor class is a better data structure to be using here, rather than enforcing object structure by convention. The downside to this is that either downstream operators (e.g. PCA, GMM, FV) would need to be changed to handle this new class (probably too big for this PR) or a conversion node would have to be invented that extracts out the matrix in the right order, which feels like a hack.

I'm going to add a test for this, and I've opened #210 to address the ImageDescriptor problem in a future PR.

etrain commented 8 years ago

Test case added.

shivaram commented 8 years ago

LGTM