add support for device selection and multiple GPUs

[mitjap]

Description

This pull request enables user to select on which GPU algorithm should run and makes it possible to run on multiple GPUs.

Features list

• Select CUDA device
• Ability to run on multiple CUDA devices

Implementation remarks

For use of multiple GPUs this implementation requires multiple PopSift instances. Main issue was that algorithm uses global state with extern variables. I made those thread_local which enables each thread to have its own value for specific device.

I have not tested matching (ProcessingMode == MatchingMode).

[mitjap]

In case you decide not to accept this PR, you should at least fix this very minor memory leak.

popsift/src/popsift/sift_pyramid.cu

Line 215 in 290e142

cudaFree( _d_extrema_num_blocks );

[griwodz]

I am not certain if the thread_local can fail for anybody. It can probably not fail for hct, hbuf and so on because those are only used in a thread spawned by PopSift.

I understand that the thread_local forces you to init the filter and the configuration in the extraction/match threads. That leads to more frequent configuration calls, is that problematic?

[griwodz]

Do I understand correct that you want to delete the Pyramid every time you have downloaded the features? That would crash if several images have been queued for feature extraction, wouldn't it?

[mitjap]

Pyramid is deleted only after pipeline stops using PopSift::uninit function. Note that private_unit (this is actually a typo and should be private_uninit) is called outside while loop.

[griwodz]

Shouldn't you add private_uninit also to the matchPrepareLoop?

[mitjap]

Yes, you are right.

[griwodz]

This looks like a good thing to do. Perhaps with an error check when a user chooses an non-existant device?

[mitjap]

I agree. Do you think using device_prop_t::set(int, bool) would be good? Another solution would be manual check with

POP_CUDA_FATAL_TEST( cudaSetDevice( currentDevice ), "Cannot set device" );

or maybe just POP_CHK.

[mitjap]

I understand that the thread_local forces you to init the filter and the configuration in the extraction/match threads. That leads to more frequent configuration calls, is that problematic?

Filter function checks if configuration differs so I don't expect it to call any CUDA functions.

Second applyConfiguration() which is within while loop is to support following usage:

PopSift sift(PopSift::ByteImages, device); // initializes with default configuration in spawned thread
sift.configure(config); // this configuration is applied when first image is enqueued.

[mitjap]

I am not certain if the thread_local can fail for anybody. It can probably not fail for hct, hbuf and so on because those are only used in a thread spawned by PopSift.

I'm sorry I don't quite understand what it is you want to say here.

[mitjap]

For more robust interface maybe there should be another call to cudaSetDevice(_device) in PopSift::uninit function so that we make sure that device images are properly deleted.

[griwodz]

I'm sorry that it took me so long before I could review the code. I hope that your improvements have been working well for you.

I have two remaining change requests before approving:
(1) I think that private_unit() should also be called at the end of matchPrepareLoop()
(2) private_uninit would be a better name than private_unit

[simogasp]

@mitjap if u can also please update CHANGES.md under v1.0.0 and add one line with the content of this PR

[griwodz]

@simogasp Should I merge this PR and follow up with the additional fixes? Can't push to the original branch.

[mitjap]

If by "pushing to original branch" you mean my branch I think I enabled pushing for popsift maintainers. I can make code changes as requested but at the moment I don't have the time to test it properly.

[griwodz]

@mitjap Thanks, I'll give it another try tomorrow. I had cloned your repo and tried to push the uninit in match and it didn't work, but I may have made some other mistake.

[mitjap]

I made the requested changes to the code.

[griwodz]

Thank you for the fixes!