Skip to content

Add tests on top of #69#78

Closed
hageboeck wants to merge 12 commits intomasterfrom
epoch2_tests
Closed

Add tests on top of #69#78
hageboeck wants to merge 12 commits intomasterfrom
epoch2_tests

Conversation

@hageboeck
Copy link
Member

@hageboeck hageboeck commented Dec 1, 2020

See PR #69.

Here, the tests for epoch1 were ported to epoch2.
The tests fail for now, since a matrix element changed by more than 1.E-9, which exceeds the test threshold of 1.E-14 (to discuss if that threshold makes sense, anyway).

@oliviermattelaer: Note that I had to rebase your commits on top of madgraph/master to make git understand what's going on. You should be able to take the diffs of my commits, and put them on top of your branch.

The good news:
The tests only needed

  #include "CommonRandomNumbers.h"
- #include "CPPProcess.h"
+ #include "gCPPProcess.h"
  #include "Memory.h"
  #ifdef __CUDACC__
  #include "grambo.cu"

The bad news (@roiser):
The downsides of the epochIsDirectory now shows:
Porting the test files and makefiles was a bit painful, because you have to trick git. You need something like

git show <commit hash of commit in ep1> | git am -p 4 -3

This

  1. Gets the changes from that commit
  2. (-p 4) Prunes the first 4 directories from the path, so epoch1 is gone.
  3. (-3) Tries to apply a three-way merge. That's always necessary, because git won't let the merge happen, because it thinks that it's not the same file (which is true).
  4. If lucky, the commit applies or you need to fix the files manually. Half of them applied, half had to be edited.
  5. Then it commits with the original message. That's nice!
  6. Then I re-edited the commit message to add info about the original commit this came from.

TODO:

  • Move common test infrastructure into something like test/
  • Agree on reference for all (?) epochs
  • Test thresholds?

@oliviermattelaer
Copy link
Member

Excellent, Thanks a lot,

For the physics point of view, I would indeed not worry about relative difference of the order of 1e-7.
But this might shows that we can be quite sensitive to numerical accuracy.

So for me this is good to go.

@hageboeck
Copy link
Member Author

@oliviermattelaer does this mean that I should raise the test thresholds for MEs to 1.E-8 or so?
Do you want to port the commits I added to madgraph or will you port these things in epoch3?

oliviermattelaer added a commit that referenced this pull request Dec 2, 2020
@oliviermattelaer
Copy link
Member

oliviermattelaer commented Dec 2, 2020

@hageboeck, Yes I think that a larger threshold makes sense.

Do you want to port the commits I added to madgraph or will you port these things in epoch3?

I have just included the commit into the epoch2 branch. Tell me if I have miss some of the changes.

Thanks,

Olivier

@valassi
Copy link
Member

valassi commented Dec 3, 2020

Hi, thanks all! Just one comment about the tests: we should separate them into two sets of object files and executables, as we do now for the main executable check.exe and gcheck.exe. Mixing c++ and cuda can give issues, for instance I had some while adding multithreading (using Stephan's suggested omp), which otherwise does work nicely out of the box! See #82 and #83 .

oliviermattelaer and others added 12 commits December 9, 2020 16:38
Port 52804e5 to epoch2.

Reorganise Makefiles to prepare it for addition of tests.
- Remove hard-coded CUDA paths. Instead, react to CUDA_HOME if exported.
- Replace "cd xxx && make" by the more standard "make -C".
- If no nvcc is found and no MGONGPU_CONFIG is given, automatically switch to C++ random numbers.
This allows for compiling the CPU-only code.
- Separate optimisation flags from other C++ flags. This facilitates
switching between debug and optimised builds.
- Protect all cuda-related variable with a conditional, so the
corresponding targets don't get built.

Fixes #55.
In order to be able to set e.g. -DMGONGPU_CURAND_ONHOST etc from
outside, they must not be overwritten in mgOnGpuConfig.h
Therefore, the section that sets one of those macros is now protected
such that it only has an effect if none of these macros is set.

Ported to epoch2 from b52f8f2
Depending on which combination of preprocessor defines were set, one
memcpy would not compile (compile with common random numbers and without
cuda).

Ported to epoch2 from 150d0ec
Using 'make info', one can now trigger printout like

/usr/local/cuda-11.1//bin/nvcc --version
nvcc: NVIDIA (R) Cuda compiler driver
Copyright (c) 2005-2020 NVIDIA Corporation
Built on Mon_Oct_12_20:09:46_PDT_2020
Cuda compilation tools, release 11.1, V11.1.105
Build cuda_11.1.TC455_06.29190527_0

g++ --version
g++ (GCC) 8.3.1 20191121 (Red Hat 8.3.1-5)
Copyright (C) 2018 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
Port 9589c50
Add a test that runs the CPU workflow, and compares with a reference file.
The reference file was created using check.exe on Centos8 with common
random numbers.

Port 719e4fc
Implement CUDA equivalent of CPU test.

- Implement compilation of CPU and GPU code into one exe.
@hageboeck
Copy link
Member Author

@hageboeck, Yes I think that a larger threshold makes sense.

Do you want to port the commits I added to madgraph or will you port these things in epoch3?

I have just included the commit into the epoch2 branch. Tell me if I have miss some of the changes.

@oliviermattelaer Threshold changed, CPU tests passing!
We should talk tomorrow how to merge this.

@hageboeck
Copy link
Member Author

Closing in favour of #109

@hageboeck hageboeck closed this Dec 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants