Remove gen_ximprove.py and madevent_interface.py from patch.common?

[valassi]

Hi @oliviermattelaer this is a followup to your comments in PR #798:
#798 (comment)
#798 (comment)

Essentially, you suggest that patch.common should not touch madevent_interface.py, do I understand this correctly?

Now, this issue is already in upstream/master. My changes in PR #798 modify the patch, but for the moment I have no way of doing that differently. I suggest to strip that off PR #798 and solve it in a separate PR for this new issue.

Note also, this is not strictly a new issue: you already mentioned this in #756 and specifically #756 (comment)

python file modify by patch (create issue with launch)
- [x] banner.py (no need)
   - [x] add parameter in the run_card (fixed via generic method)  
- [ ] gen_ximprove.py
   - [ ]  change compile code
- [ ] madevent_interface.py
   - [ ] change compile code 

By the way, I suspect that these changes were introduced here (cc @roiser who may know more)

commit 2a188c04d945726b5f2ac071a750360e3ec303c3
Author: Stefan Roiser <stefan.roiser@cern.ch>
Date:   Fri Mar 31 18:07:55 2023 +0200

    add compile for individual targets in the refine step

diff --git a/epochX/cudacpp/gg_tt.mad/bin/internal/madevent_interface.py b/epochX/cudacpp/gg_tt.mad/bin/internal/madevent_interface.py
index 9a308cc60..50392be67 100755
--- a/epochX/cudacpp/gg_tt.mad/bin/internal/madevent_interface.py
+++ b/epochX/cudacpp/gg_tt.mad/bin/internal/madevent_interface.py
@@ -3615,8 +3615,20 @@ Beware that this can be dangerous for local multicore runs.""")
                 logger.info('    %s ' % subdir)
     
                 if os.path.exists(pjoin(Pdir, 'ajob1')):
-                    self.compile(['madevent'], cwd=Pdir)
-                    
+
+                    exec_mode = 0
+                    if 'exec_mode' in self.run_card:
+                        exec_mode = self.run_card['exec_mode']
+
+                    if exec_mode == 0:
+                        self.compile(['madeventfortran'], cwd=Pdir)
+                    elif exec_mode == 1:
+                        self.compile(['madeventcpp'], cwd=Pdir)
+                    elif exec_mode == 2:
+                        self.compile(['madeventcuda'], cwd=Pdir)
+                    else:
+                        self.compile(['all'], cwd=Pdir)
+
                     alljobs = misc.glob('ajob*', Pdir)
                     
                     #remove associated results.dat (ensure to not mix with all data)

You mention that now we can do some things with launch_plugin.py, but at the time I think that launch_plugin.py did not exist yet.

Anyway - maybe you can suggest a better way please? The current patch.common for bin/internal files in upstream/master now includes this

diff --git b/epochX/cudacpp/gg_tt.mad/bin/internal/gen_ximprove.py a/epochX/cudacpp/gg_tt.mad/bin/internal/gen_ximprove.py
index fb7efa87c..5fd170d18 100755
--- b/epochX/cudacpp/gg_tt.mad/bin/internal/gen_ximprove.py
+++ a/epochX/cudacpp/gg_tt.mad/bin/internal/gen_ximprove.py
@@ -391,8 +391,20 @@ class gensym(object):
                         done = True
                 if not done:
                     raise Exception('Parsing error in gensym: %s' % stdout)
-                     
-            self.cmd.compile(['madevent'], cwd=Pdir)
+
+            cudacpp_backend = self.run_card['cudacpp_backend'] # the default value is defined in banner.py
+            logger.info("Building madevent in madevent_interface.py with '%s' matrix elements"%cudacpp_backend)
+            if cudacpp_backend == 'FORTRAN':
+                self.cmd.compile(['madevent_fortran_link'], cwd=Pdir)
+            elif cudacpp_backend == 'CPP':
+                self.cmd.compile(['madevent_cpp_link'], cwd=Pdir)
+            elif cudacpp_backend == 'CUDA':
+                self.cmd.compile(['madevent_cuda_link'], cwd=Pdir)
+            else:
+                raise Exception("Invalid cudacpp_backend='%s': only 'FORTRAN', 'CPP', 'CUDA' are supported")
+                ###logger.info("Building madevent with ALL(FORTRAN/CPP/CUDA) matrix elements (cudacpp_backend=%s)"%cudacpp_backend)
+                ###self.cmd.compile(['all'], cwd=Pdir)
+
             if to_submit:
                 self.submit_to_cluster(job_list)
                 job_list = {}
diff --git b/epochX/cudacpp/gg_tt.mad/bin/internal/madevent_interface.py a/epochX/cudacpp/gg_tt.mad/bin/internal/madevent_interface.py
index 8c509e83f..cb6bf4ca5 100755
--- b/epochX/cudacpp/gg_tt.mad/bin/internal/madevent_interface.py
+++ a/epochX/cudacpp/gg_tt.mad/bin/internal/madevent_interface.py
@@ -3614,8 +3614,20 @@ Beware that this can be dangerous for local multicore runs.""")
                 logger.info('    %s ' % subdir)
     
                 if os.path.exists(pjoin(Pdir, 'ajob1')):
-                    self.compile(['madevent'], cwd=Pdir)
-                    
+
+                    cudacpp_backend = self.run_card['cudacpp_backend'] # the default value is defined in banner.py
+                    logger.info("Building madevent in madevent_interface.py with '%s' matrix elements"%cudacpp_backend)
+                    if cudacpp_backend == 'FORTRAN':
+                        self.compile(['madevent_fortran_link'], cwd=Pdir)
+                    elif cudacpp_backend == 'CPP':
+                        self.compile(['madevent_cpp_link'], cwd=Pdir)
+                    elif cudacpp_backend == 'CUDA':
+                        self.compile(['madevent_cuda_link'], cwd=Pdir)
+                    else:
+                        raise Exception("Invalid cudacpp_backend='%s': only 'FORTRAN', 'CPP', 'CUDA' are supported")
+                        ###logger.info("Building madevent with ALL (FORTRAN/CPP/CUDA) matrix elements (cudacpp_backend=%s)"%cudacpp_backend)
+                        ###self.compile(['all'], cwd=Pdir)
+
                     alljobs = misc.glob('ajob*', Pdir)
                     
                     #remove associated results.dat (ensure to not mix with all data)

Thanks! Andrea

[oliviermattelaer]

Hi Andrea,

Actually this should have been solved already correctly and I guess the fact that the patch is still present is just a mistake (maybe related to a merge issue). On my machine, I have simply remove those patches and it works as it should.
Maybe this depend on the exact workflow (I tested two separate ones) ...
So i have propose a MR (#849) where those patches are simply removed to check the CI.

I propose that we already close this thread and finish this discussion on the MR. (and will check old thread to update the information, thanks for the link) --off course do not hesitate to re-open this thread if you think this is relevant--.

[valassi]

Reopened just to link it to #849 and #939, which have been merged a few days ago. Closing.