TheCodez / dynamic-occupancy-grid-map

Implementation of "A Random Finite Set Approach for Dynamic Occupancy Grid Maps with Real-Time Application"
MIT License
281 stars 39 forks source link

clang-tidy on cuda files #64

Closed cbachhuber closed 4 years ago

cbachhuber commented 4 years ago

Currenty, this is a backup solution: run clang-tidy on cuda files, but do not treat warnings as errors. This way, clang-tidy issues (including current compilation errors) will be reported in CI, but CI will not fail upon them.

I want to try to get a proper solution working, people have done this before. I expect that we need to adjust something in the CMakeLists.txt. I'll try to set up a clean 'hello world' cuda project and run clang-tidy on that.

Btw: you might want to install https://github.com/apps/wip in this repository ;)

TheCodez commented 4 years ago

Cool, nice seeing progress here. I installed wip. Is there any significant difference to creating a draft pr?

TheCodez commented 4 years ago

Fixed some of the warnings in https://github.com/TheCodez/dynamic-occupancy-grid-map/commit/2adf1c154038715e03307428fef2771eeb8aadb1 Most are related to some code in the thrust library.

cbachhuber commented 4 years ago

I installed wip. Is there any significant difference to creating a draft pr?

Thanks! Good point, I've never tried draft PRs. From what I can see, they're basically the same thing. Now this repo supports both 😅

cbachhuber commented 4 years ago

I finished my experiments on clang-tidy + CUDA: for clang-tidy to work on .cu files without compilation errors, you need to point it to your cuda installation via --cuda-path=. That is only supported for cuda libraries installed from run packages.

Such a package can be installed on a GPU-less machine as well, however it is then pointing to some faulty GPU architecture which would require a very old cuda version (which conflicts with Ubuntu 18.04). This old architecture is in my case not supported by clang, so I would need a dedicated GPU to support all this.

Long story short: I don't think that we can run clang-tidy on cuda code in CI as gating check. At least not without complex, dirty hacks on CUDA and clang-tidy. Therefore, I would leave it as it is: a non-gating check that at least gives you warnings. What do you think?

TheCodez commented 4 years ago

Thanks for taking the time and doing the experiments. Just having warnings is sufficient and definetly better than nothing :)