search

ethz-asl / ethzasl_brisk

Brisk. The feature.
20 stars 11 forks source link

Feature/brisk2.0 camera aware #22

Open simonlynen opened 10 years ago

simonlynen commented 10 years ago

This is a PR to keep track of things to do before merging the camera-aware extraction to master.

mbuerki commented 10 years ago

can we merge this?

simonlynen commented 10 years ago

@mbuerki It looks like this PR got completely out of scope. :(

Can you fix the build, I can then have a look through.

simonlynen commented 10 years ago

@mbuerki also you probably have to merge master into this branch to get it into a mergable state.

mbuerki commented 10 years ago

it fails now tbecause there are unused paramter warnings (which become errors). what is the procedure for function calls with unused parameters? I think there was a discussion about that some while ago but I'm not sure to what conclusion it came.

HannesSommer commented 10 years ago

The conclusion was to have them commented out : e.g. , 

void bar(int /*foo*/ ) {}

If they are used or not depending on macros (like NDEBUG) there is the option to use a 

static_cast<void>(foo);

I would then write a short comment behind why it is there.

mbuerki commented 10 years ago

Jenkins test this please

mbuerki commented 10 years ago

ok, now this compiles but a harris verification test fails (eventually, the check in bench-df.h line 356 fails)

any insight on that?

simonlynen commented 10 years ago

@mbuerki This is a binary verification test that checks that the algorithm has not been altered unintentionally. It is currently not possible to tell if the changes you did are expected to change this particular result or if this is a bug. Lets try to see what happens once you cut the PR up.