Bug in YoloV5Layer implementation

[Armassarion]

In the enqueue method of the yolo layer implementation, you should use cudaMemsetAsync or the result might be wrong with big batch size of parallel processing.

[wang-xinyu]

@Armassarion Hi, why is that? Can you explain a bit more?

[Armassarion]

The current implementation, considering the main, is correct.
But if you use multiple TRT inference context (engine->createExecutionContext()) and different cudaStream with each, you'll potentially face a race condition on this part CUDA_CHECK(cudaMemset(output + idx * outputElem, 0, sizeof(float))); as it may memset the value at the wrong time (parallel read/write without lock). Typically, let's say you remove the cudaStreamSynchronize (L261 @ yolov5.cpp) to enable asynchronous processing (for example, using cuda events). You'll launch multiple inference on the same context (and that's valid) but each enqueue will reset the number of detections instead of waiting for the previous inference to finish and be downloaded.

To fix that (without impacting the current implementation), I suggest to use CUDA_CHECK(cudaMemsetAsync(output + idx * outputElem, 0, sizeof(float), stream));

The stream is already passed to the kernel call (CalDetection <<<(numElem + mThreadCount - 1) / mThreadCount, mThreadCount, 0, stream>>>) so it would be more consistent to also use it in the memset operation.

[wang-xinyu]

OK, got it, so it will cause issue when launching multiple context.

Can you raise a PR to fix it?

[Armassarion]

I need to be able to push a branch to this repo to do a PR. I think I don't have this right

[wang-xinyu]

You can fork it firstly and then commit to that and then PR.

[Armassarion]

Sorry, I didn't really know how github works.
It's done :)

[wang-xinyu]

linking to #720

[MichaelWU0726]

Sorry, I didn't really know how github works. It's done :)

you only need to click the button fork in the left-up and fix yourself repo, then submit a PR from your repo to this repo

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.