Closed lukasmerten closed 2 years ago
This PR is now ready for review and feedback.
There are probably many things that can be improved. With the 3.2 release in mind I tried to focus on testing only. Deployment of documentation etc. could come in a later step.
The macOS implementation fails at the moment I have no real clue why. It seems to be related to the downloaded data. Maybe someone with mac experience has an idea.
The preinstalled swig for ubuntu20 is still not updated to 4.0.2 --> Swig_builtin is deactivated. Either we try to manually install the latest swig version or wait until the github actions virtual environments are updated.
I'll take a look at running the coverage report and deploying the documentation in the future. However, from my point of view this should not stop us from continuing with this PR.
Hi @lukasmerten Indeed, this doesn't work on OSX. I managed to fix it with the code below (around line 355).
if(EXISTS ${CMAKE_BINARY_DIR}/data-${CRPROPA_DATAFILE_VER}.tar.gz)
execute_process(COMMAND "tar -xzf ${CMAKE_BINARY_DIR}/data-${CRPROPA_DATAFILE_VER}.tar.gz" WORKING_DIRECTORY ${CMAKE_BINARY_DIR})
execute_process(COMMAND "cp -r ${CMAKE_BINARY_DIR}/data-${CRPROPA_DATAFILE_VER}/data ${CMAKE_BINARY_DIR}/data/." WORKING_DIRECTORY ${CMAKE_BINARY_DIR})
execute_process(COMMAND "rm -r ${CMAKE_BINARY_DIR}/data-${CRPROPA_DATAFILE_VER}/" WORKING_DIRECTORY ${CMAKE_BINARY_DIR})
Just don't ask me why this works and the previous implementation doesn't.
Hi @rafaelab When I implement your suggestion none of the jobs is passing. Also locally (on Ubuntu 18) your solutions does not work. Re-implementing the older solution with an explicit copy (cp -r) command works on ubuntu, too.
@lukasmerten Could you please test this?
execute_process(COMMAND ${CMAKE_COMMAND} -E tar xzf ${CMAKE_BINARY_DIR}/data-${CRPROPA_DATAFILE_VER}.tar.gz WORKING_DIRECTORY ${CMAKE_BINARY_DIR})
execute_process(COMMAND ${CMAKE_COMMAND} -E copy_directory ${CMAKE_BINARY_DIR}/data-${CRPROPA_DATAFILE_VER}/data ${CMAKE_BINARY_DIR}/data/ WORKING_DIRECTORY ${CMAKE_BINARY_DIR})
execute_process(COMMAND ${CMAKE_COMMAND} -E remove_directory ${CMAKE_BINARY_DIR}/data-${CRPROPA_DATAFILE_VER}/ WORKING_DIRECTORY ${CMAKE_BINARY_DIR})
It works for me on OSX.
I guess the problem is somehow related to setting the CRPROPA_DATA_PATH environment variable for the macos test. I guess I am doing a stupid syntax mistake here. If anyone wants to check the logfiles from the test runs they can be found here.
Let me check this in a simpler setup and I hope to get back to this with some solution today.
I now hardcoded the path for the CRPROPA_DATA_PATH, which seems to fix most the of the issue with macos. Probably not a good solution but fixes it for now. The macos job fails now only in one test of the interaction:
[ RUN ] EMPairProduction.secondaries unknown file: Failure C++ exception with description "vector" thrown in the test body. [ FAILED ] EMPairProduction.secondaries (57 ms)
@rafaelab Any idea were this might come from?
[ RUN ] EMPairProduction.secondaries unknown file: Failure C++ exception with description "vector" thrown in the test body. [ FAILED ] EMPairProduction.secondaries (57 ms)
@rafaelab Any idea were this might come from?
I'm not sure, but I think this is related to the OSX native clang compilers. I never encountered this problem before using GCC. I think the solution is to limit the next step value to something other than numeric_limits<double>
.
So we have to replace
c.setNextStep(std::numeric_limits<double>::max());
with
c.setNextStep(1e4 * Mpc);
I don't know why this only happens for EMPairProduction.
Tests for EMInverseComptonScattering are also extremely slow (~5 minutes). I will investigate this further, but since it is not critical I think we can just leave it for the time being.
I changed to c.setCurrentStep(1e10 * Mpc)
(line 682 in testInteraction), which was supposedly used in some earlier version of the code. However, this does not fix the problem.
Hi @lukasmerten
It seems that the gcc set up is making it impossible to compile CRPropa (at least on my machine). I just ran git pull
and tried to compile your PR in exactly the same way as I did last week. It throws a bunch of compiler-related errors when running cmake ..
Is commit 077965c still working for you? After that I only changed things in the file to set up the automatique tests, which should have no influence on any local installation.
Remotely, the three linux setups and macos with clang are working now. The missing part is macos on gcc at the moment.
I give up on the macos gcc combination. If someone has exact instruction how to set this up in github action I am happy to update this PR.
There are now working tests on ubuntu 18/20 with python 2/3 and macos using clang compilers. I checked and we always used clang for the CI of macos on travis-ci. Also runs with and without SIMD extension and SWIG built-in feature are included.
Hi @lukasmerten
The compilation error I reported earlier was a bug with my system after an update that I completely forgot about. Sorry about that!
But this was actually helpful. Many things stopped working and I reinstalled a few things. I don't really understand why, but now all tests pass with both Clang and GCC9. (I tested testInteraction
10 times with each compiler).
@rafaelab do you want to include an automated macos + gcc test or are you just fine with the existing macos + clang test?
I'm fine with the default clang test. Are you done with this PR, @lukasmerten? If yes, I will review it tomorrow.
It's ready from my point of view.
Hi @lukasmerten I'm still reviewing the PR. I'll only be able to finish it tomorrow evening. So far it looks good. However, I can't see the right badge for the build status.
The build status badge is not visible because I already linked it to the CRPropa github actions, but the tests in this PR are performed on my account. I should work, when the PR is merged.
Starting point for a possible moving of our CICD to github actions.
At the moment, it looks like github offers free computation time for open source projects. Since I have a not found any other (easy to use and free) option I started to move our CICD to github actions.
Current status:
To Do: