Fix missing references in GooFit python bindings

[FlorianReiss]

Changes to CMake to fix issues with missing references when using GooFit python bindings. Based on changes by @danielsibemol

On sneezy, the following setup should work for compiling GooFit

ml load gcc/7.5/cuda/11.2
source /usr/local/anaconda3/etc/profile.d/conda.sh
cmake -S . -B build-cuda -DGOOFIT_FORCE_LOCAL_THRUST=OFF -DGOOFIT_CERNROOT=OFF -DGOOFIT_PYTHON=ON -DGOOFIT_KMATRIX=ON -DGOOFIT_EXAMPLES=OFF -DGOOFIT_TESTS=OFF
cmake --build build-cuda/ -j 24

to use python bindings:

export PYTHONPATH=/share/lazy/freiss/GooFit/build-cuda (change accordingly)

Notes:

• it might be that not all of the changes are necessary
• after changing something, the build directory needs to be cleaned before re-compiling for the linking to work. Probably there are other changes to CMake required to fix
• needs adjustment in MCBooster external repository to compile

@danielsibemol

[henryiii]

This should already be set, I believe. Also, it should be set only for CUDA backend (which I think it already is).

[FlorianReiss]

you are right, this was already set and this line is not needed

[FlorianReiss]

@henryiii after the changes here, the linking does not seem to be working when changing something and trying to recompile with errors like

nvlink error   : Undefined reference to '_ZN6GooFit12phsp_twoBodyEddd' in '../src/PDFs/physics/libPDFPhysics.a:kMatrix.cu.o'
nvlink error   : Undefined reference to '_ZN6GooFit11phsp_fourPiEd' in '../src/PDFs/physics/libPDFPhysics.a:kMatrix.cu.o'
nvlink error   : Undefined reference to '_ZN6GooFit13getPropagatorEPA5_KdPKN6thrust7complexIdEEPA5_S5_d' in '../src/PDFs/physics/libPDFPhysics.a:kMatrix.cu.o'
python/CMakeFiles/_goofit.dir/build.make:151: recipe for target 'python/CMakeFiles/_goofit.dir/cmake_device_link.o' failed

It is then necessary to clean the build directory and recompile from scratch. Do you know how one could fix this?

[FlorianReiss]

@henryiii @danielsibemol do you have any ideas how to properly do this, since addingCUDA_RESOLVE_DEVICE_SYMBOLS ON isn't a good solution?

[henryiii]

Why can't we just make it optional when building; in fact, if we just pass -DCUDA_RESOLVE_DEVICE_SYMBOLS=ON when configuring, I think no changes are needed at all, save to documentation?

[henryiii]

No, I take that back. This is only a property target: https://cmake.org/cmake/help/latest/prop_tgt/CUDA_RESOLVE_DEVICE_SYMBOLS.html Also, it seems it is doing something slightly different than I thought. As long as it doesn't break GPU-less compilation (which it doesn't since the CI passes), I think it's okay. Sorry for flip-flopping on this! Okay to merge if you are.

[FlorianReiss]

No, I take that back. This is only a property target: https://cmake.org/cmake/help/latest/prop_tgt/CUDA_RESOLVE_DEVICE_SYMBOLS.html Also, it seems it is doing something slightly different than I thought. As long as it doesn't break GPU-less compilation (which it doesn't since the CI passes), I think it's okay. Sorry for flip-flopping on this! Okay to merge if you are.

before this is merged, there is an issue with recompiling with these changes which would be good to understand. When modifying some code (e.g. source code of some PDF) and recompiling with python bindings, a linking issue appears. The only solution I found was to clean the build directory and compile from scratch, after which the compilation succeeds