Remove dependency on cub

[VinInn]

This PR build on #447 and remove dependencies from cub in the allocator as well.
BuildFiles have been cleaned.

In principle cub can be removed as required external.

test passed

[fwyzard]

I don't understand... here is what GitHub shows me:

[VinInn]

that was one additional commit due to conflicts (I was in pre5)
this is what I see

[fwyzard]

@VinInn could you rebase this on top of the latest CMSSW_11_1_X (which now includes #447) ?

[VinInn]

looks ok but includes explicitly #447. Any suggestion?

[fwyzard]

looks ok but includes explicitly #447. Any suggestion?

Do you mean the it appears in the diff ?
Let me see if re-targetting the PR helps with that.

[fwyzard]

Nope.

[fwyzard]

According to a discussion on GitHub forums it's not a bug, it's a feature:

When you merge commits from a Pull Request into a branch, the commit SHA changes from what the original commit SHA's were.

This means when you compare the same two branches again, the diff will show those commits again because those specific commit SHA's don't exist on the branch you're merging into.

This is a result of the type of diff we use on GitHub. We use git's three dot diff, which is the difference between the latest commit on the HEAD branch and the last common ancestor commit with the base branch.

This matches what I see locally:

normal git diff with two dots ..

$ git diff CMSSW_11_1_X_Patatrack..pull/449 -- HeterogeneousCore/CUDAUtilities/interface/HistoContainer.h
[nothing]

git diff with three dots ...

$ git diff CMSSW_11_1_X_Patatrack...pull/449 -- HeterogeneousCore/CUDAUtilities/interface/HistoContainer.h
diff --git a/HeterogeneousCore/CUDAUtilities/interface/HistoContainer.h b/HeterogeneousCore/CUDAUtilities/interface/HistoContainer.h
index 7b31863a9f4c..bd80281f7533 100644
--- a/HeterogeneousCore/CUDAUtilities/interface/HistoContainer.h
+++ b/HeterogeneousCore/CUDAUtilities/interface/HistoContainer.h
@@ -9,10 +9,6 @@
 #include <cstdint>
 #include <type_traits>
 
-#ifdef __CUDACC__
-#include <cub/cub.cuh>
-#endif
-
 #include "HeterogeneousCore/CUDAUtilities/interface/AtomicPairCounter.h"
 #include "HeterogeneousCore/CUDAUtilities/interface/cudaCheck.h"
 #include "HeterogeneousCore/CUDAUtilities/interface/cuda_assert.h"
@@ -61,32 +57,31 @@ namespace cms {
                                                           = cudaStreamDefault
 #endif
     ) {
-      uint32_t *off = (uint32_t *)((char *)(h) + offsetof(Histo, off));
+      uint32_t *poff = (uint32_t *)((char *)(h) + offsetof(Histo, off));
+      int32_t size = offsetof(Histo, bins) - offsetof(Histo, off);
+      assert(size >= int(sizeof(uint32_t) * Histo::totbins()));
 #ifdef __CUDACC__
-      cudaCheck(cudaMemsetAsync(off, 0, 4 * Histo::totbins(), stream));
+      cudaCheck(cudaMemsetAsync(poff, 0, size, stream));
 #else
-      ::memset(off, 0, 4 * Histo::totbins());
+      ::memset(poff, 0, size);
 #endif
     }
 
     template <typename Histo>
[...]

So, it looks like the only options for cleaning up the diff are to

• rebase instead of merging
• squashing all commits

The rebase cannot be done automatically, so I would not suggest doing it.

The squash is as simple as

git checkout RemoveDependencyFromCUB
git merge CMSSW_11_1_X_Patatrack -m 'Merge'
git reset CMSSW_11_1_X_Patatrack
git add -A .
git commit -m'Remove all dependencies on cub'
git push -f

(if anybody knows a simpler way, I'll be happy to hear it)

However, in out case this affects only the diff shown on GitHub: when we merge we squash all commits anyway, so the end results will be the same.

[VinInn]

Is it technically possible to run 'code format' directly on git?

[fwyzard]

Not as far as I know.

[fwyzard]

Looks good for merging, waiting just to give @makortel a chance to comment.

[makortel]

If all errors are handled with cudaCheck() internally, I'd expect there is no need to return an "error code" anymore (that was already passed through cudaCheck() in the calling code). Something to be cleaned up in a subsequent PR though.

[fwyzard]

Since we don't expect errors, I would just cudaCheck all function calls (except for retrying the out-of-memory case).
Can be followed up later.

[makortel]

Since we don't expect errors, I would just cudaCheck all function calls (except for retrying the out-of-memory case).
Can be followed up later.

I agree.

[makortel]

This is a functional change, earlier it would print out the error code if CUB_STDERR was defined (which IIRC we do define). Doesn't really matter to me though.

[makortel]

The caching allocators would benefit from further cleanup (well, rewrite from scratch really...), but better to be done later.

[makortel]

@fwyzard Thanks for annotating all the changes, I think this PR is good to go now.