Initialize tracks on device

[amandalund]

Add some of the functionality for initializing tracks on device from primaries and secondaries in a way that is reproducible.

[sethrj]

First round of feedback. The core functionality looks good, but the way all the classes integrate is still kind of fuzzy (by necessity, because we don't yet have a high-level plan for how the pieces fit together). How about we talk on Monday's infrastructure meeting about converging on a high-level starting point plan that we can adapt as time goes on?

[amandalund]

Thanks for looking this over! It’s definitely still pretty rough and missing some pieces — I was hoping we could go over some things before I went much further with it, so that sounds great.

[sethrj]

Thanks for looking this over! It’s definitely still pretty rough and missing some pieces — I was hoping we could go over some things before I went much further with it, so that sounds great.

@amandalund For sure, and thanks for the hard work! If Monday's infrastructure meeting is too late (or if you want to do a one-on-one rather than have to have other people watching) I'm free from noon to 3ish eastern time.

[sethrj]

@amandalund This is looking great. I had to make a few changes (to fix the build when CUDA is disabled, I wrote stubs for the curand code) that I can move to another branch, and I did a tiny bit of restructuring (moving files to a "detail" namespace). Should I just push those changes on top of your branch? Some of the changes I could put into a separate MR if you'd prefer.

[amandalund]

Awesome, thanks @sethrj! You can just push them here.

[sethrj]

Amanda, this was a huge lift and looks excellent. The higher-level data organization is still a little muddled (as much my fault as it is yours) so let's talk Monday about how to organize the different primary/secondary/track states.

[dalg24-jenkins]

Can one of the admins verify this patch?

[sethrj]

add to whitelist

[sethrj]

A couple more minor things sorry! 😔 It's looking really good though 😄 and these should be quick fixes.

[sethrj]

I squashed the branch's history, all tests pass. Excellent work, @amandalund !

[sethrj]

Related to #119 @pcanal : the reason @amandalund had to add the separable compilation linker flags to the main celeritas library (in src/CMakeLists) was she hit the following build error:

[1/56] Building CUDA object src/CMakeFiles/celeritas.dir/sim/InitializeTracks.cu.o
FAILED: src/CMakeFiles/celeritas.dir/sim/InitializeTracks.cu.o 
/usr/local/cuda-10.2/bin/nvcc -forward-unknown-to-host-compiler -ccbin=/usr/bin/g++ -DVECCORE_ENABLE_CUDA -Dceleritas_EXPORTS -I/projects/spack/opt/spack/gcc-8.3.1/clhep/ual4i4i/lib/CLHEP-2.4.1.3/../../include -I../src -Isrc -isystem=/projects/spack/var/spack/environments/celeritas/.spack-env/view/include -isystem=/usr/local/cuda-10.2/include -isystem=/projects/spack/opt/spack/gcc-8.3.1/xerces-c/ffb3b3s/include -g -G --generate-code=arch=compute_35,code=[compute_35,sm_35] -Xcompiler=-fPIC -std=c++14 -MD -MT src/CMakeFiles/celeritas.dir/sim/InitializeTracks.cu.o -MF src/CMakeFiles/celeritas.dir/sim/InitializeTracks.cu.o.d -x cu -c ../src/sim/InitializeTracks.cu -o src/CMakeFiles/celeritas.dir/sim/InitializeTracks.cu.o
ptxas fatal   : Unresolved extern function '_ZN7vecgeom20globaldevicegeomdata25GetCompactPlacedVolBufferEv'

I told her I'd seen a nearly identical error when writing the demo rasterizer the first time; my solution had been to add

  set_target_properties(demo_rasterizer_cuda PROPERTIES
    LINKER_LANGUAGE CUDA
    CUDA_SEPARABLE_COMPILATION ON
    POSITION_INDEPENDENT_CODE ON
  )

[pcanal]

So the difference between the CI and my environment turned out to be ... the version of cmake.

I was using v3.15.4 which does not support yet the flag CMAKE_CUDA_ARCHITECTURES (Noticed that to the warning from cmake "this variable was set but not used").

When I tried with cmake v3.19.3 without adding that flag, the build failed (missing symbol). This is likely due to the fact that this version of cmake no longer propagate the -arch=sm_70 set by (I think VecGeom) and instead use default value (which turns out to be sm_52). Once I added -DCMAKE_CUDA_ARCHITECTURES:STRING=70, the build succeeded and the process succeeds ....

Bottom line, we need to require a version of cmake "greater than" v3.15.4. v3.19.3 and which ever version the CI is using seems to be acceptable version.

[sethrj]

Took me a second, but this makes sense -- with a "default value" (where does that come from? the login node on which celeritas was built, rather than the compute node?) of sm_52 the kernels that don't call vecgeom are fine, since cuda recompiles for the higher architecture at runtime based on the PTX. For the kernels that link against vecgeom I guess separable compilation gets in the way and causes the two different sets of flags to misbehave.

The CMAKE_CUDA_ARCHITECTURES flag is new as of 3.18. It seems that this error might crop up in other circumstances. Regarding an old cmake ignoring the variable, that's an easy fix:

if((CMAKE_VERSION VERSION_LESS 3.18) AND (DEFINED CMAKE_CUDA_ARCHITECTURES))
  # error
endif()

but are you sure the cuda flags will propagate correctly on older versions of cmake, or that the build works on newer versions of cmake if the architecture flags aren't set consistently between vecgeom and celeritas? This sounds like a headache...

[pcanal]

but are you sure the cuda flags will propagate correctly on older versions of cmake

It only seems to since in the end the build is not correct (see the "invalid kernel" error).

or that the build works on newer versions of cmake if the architecture flags aren't set consistently between vecgeom and celeritas?

Even with cmake 3.19, if the flags are inconsistent it wont work ... either visibly (missing symbol) or plausibly (but did not observe that) the same way it fails with cmake 3.15.

What I think we need is:

if((CMAKE_VERSION VERSION_LESS 3.18))
  # error
else()
  somehow check that the request architecture (either CMAKE_CUDA_ARCHITECTURES or whereever my sm_52 came from) match the one from VecGeom ... and if not error out.
endif()

[mrguilima]

Just upgrading CMake to 3.19.3 was not sufficient, my CMake run fails:

CMake Deprecation Warning at external/googletest/CMakeLists.txt:4 (cmake_minimum_required):
  Compatibility with CMake < 2.8.12 will be removed from a future version of
  CMake.
  Update the VERSION argument <min> value or use a ...<max> suffix to tell
  CMake that the project does not need compatibility with older versions.

How should googletest be upgraded?

[sethrj]

@mrguilima That's a warning, not an error. I'll see if that version can be updated. An easy way to avoid the error is to preinstall googletest and link to it externally by providing it in your cmake prefix path.