Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

generated code must be relocatable (must build outside the madgraph4gpu project) #613

Closed
valassi opened this issue Mar 29, 2023 · 4 comments · Fixed by #662, #657 or #666
Closed

generated code must be relocatable (must build outside the madgraph4gpu project) #613

valassi opened this issue Mar 29, 2023 · 4 comments · Fixed by #662, #657 or #666
Assignees

Comments

@valassi
Copy link
Member

valassi commented Mar 29, 2023

This is an essential issue for the release, which I gave for granted (in point 3 of the workplan #576) but probably I did not write down explicitly: the generated code must be relocatable. ie it muct build outside the madgraph4gpu project

Presently, instead, there are several things that we take from the top of the madgraph4gpu project. Two notable examples

  • the common random numbers
  • the googletest infrastructure

I guess in particular we could

  • move the common random header inside each and every process directory
  • move any pending test headers inside each and every process directory (already partially done at the time of google test and CI - ggttgg test results (and test directory restructuring) #268 for MadgraphTest.h)
  • develop a way to allow a SINGLE external googletest directory (as is the case now), possibly involving symlinks... I do NOT want to have every single build redownload and reconfigure google test

Thanks @zeniheisser for reporting the issues related to this!

@valassi
Copy link
Member Author

valassi commented May 23, 2023

Hi @zeniheisser this is related to the iussue we just discussed about common random numbers....

valassi added a commit to valassi/madgraph4gpu that referenced this issue May 23, 2023
valassi added a commit to valassi/madgraph4gpu that referenced this issue May 23, 2023
@valassi
Copy link
Member Author

valassi commented May 23, 2023

The first point in this issue (CommonRandomNumbers.h) is fixed by MRs #657 and #662

The issue remains open as we still need to fix googletest

valassi added a commit to valassi/madgraph4gpu that referenced this issue May 24, 2023
…y - this is still needed by epochX/sycl (and by epoch1/2)

This reverts parts of MRs madgraph5#663 and madgraph5#664 - it is related to madgraph5#613

Revert "removed superfluous copy of CommonRandomNumbers.h from tools directory. now lies in CODEGEN directory, is included in process generation"
This reverts commit c7136a3.
valassi added a commit to valassi/madgraph4gpu that referenced this issue May 24, 2023
…a_sm_gg_ttx.txt and modify epoch_process_id.h to read it from there

This makes the runTest.exe application more relocatable, independent of madgraph4gpu (madgraph5#613)
valassi added a commit to valassi/madgraph4gpu that referenced this issue May 24, 2023
…a_sm_gg_ttx.txt and modify epoch_process_id.h to read it from there

This makes the runTest.exe application more relocatable, independent of madgraph4gpu (madgraph5#613)
@valassi
Copy link
Member Author

valassi commented May 25, 2023

I started work on a WIP MR #666 for googletest.

There are a couple of decisions to take there, for builds outside madgraph4gpu and for new processes

  1. The tests in runTests include minor process-independent tests but especially a process-specific test that depends on the existence of a reference file. The reference currently exists for the processes we have in the cudacpp repo (and a selected few more), but clearly this will NOT be produced for every new process. Currently the test fails if the reference file does not exists. So things to decide are for instance, what to do for new processes?
  • Should the test app keep failing if the ref is not found, or should it issue a warning? This is probably irrelevant as somehow we will not run the test routinely, see below. I would keep the error: a test with no ref is useless anyway, we will probably not run it or not even build it!
  • My feeling is that we will only routinely use and run runTests,exe inside madgraph4gpu, but not outside. If at some point we add a new process to madgraph4gpu as something to focus on, we will probably create a reference file for it: so inside madgraph4gpu we should alway build the test, and have an error if the ref does not exist. Outside madgraph4gpu, conversely, we should probably not build the tests if there is no ref file, and we may even decide not to build the tests at all unless for special cases?
  1. What should we be doing with googletest outside madgraph4gpu?
  • Inside madgraph4gpu, for local builds we can have a single googletest installation. This is convenient, we only need to donwload, run configure an dthen make once, and forget about it. This should be kept, even if it "pollutes" the installation of gridpacks outside madgraph4gpu.
  • Outside madgraph4gpu, probably installing googletest and building runTest should be optional? (See also the process-specific comments above). This should still be possible, but not a default.

Maybe some decision chain like this?

  • Check somehow (directory names?) if you are inside madgraph4gpu. In that case, use a central googletest in the repo, and build runTest.
  • If outside madgraph4gpu, by default do not install googletest and do not build tests.
    • Maybe if GTEST_ROOT is set, use that as external installation?
    • Otherwise rely ona specifc google test path set inside the makefile (better than env variable)?

Essentially: if you are outside madgraph4gpu and want to build the test, you need to modify manually the makefile? Probably safe and useful enough. (Eventually some of this can be at code generation? overkill, not really neededd)

valassi added a commit to valassi/madgraph4gpu that referenced this issue May 30, 2023
…a_sm_gg_ttx.txt and modify epoch_process_id.h to read it from there

This makes the runTest.exe application more relocatable, independent of madgraph4gpu (madgraph5#613)
valassi added a commit to valassi/madgraph4gpu that referenced this issue Jun 1, 2023
valassi added a commit to valassi/madgraph4gpu that referenced this issue Jun 1, 2023
valassi added a commit to valassi/madgraph4gpu that referenced this issue Jun 1, 2023
@valassi
Copy link
Member Author

valassi commented Jun 1, 2023

This is now essentially complete in MR #666

@valassi valassi linked a pull request Jun 1, 2023 that will close this issue
valassi added a commit to mg5amcnlo/mg5amcnlo_cudacpp that referenced this issue Aug 16, 2023
valassi added a commit to mg5amcnlo/mg5amcnlo_cudacpp that referenced this issue Aug 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment