remove operation that should not be in patchmad

[oliviermattelaer]

In this case,

This is the creation of the files which should be done together with the other files in output.py.

My only concern is the definiton of the variable "${dir_patches}" which to my understanding can only take one value... If this is not the case, then this merge is likely to be refused and I would need to understand how to adapt the file.

[oliviermattelaer]

Not sure why mg5amc is marked has changed, this is irrelevant for this MR. (so that part can be discarded before merging). Looks like I do not manage yet those submodule... sorry for that

[oliviermattelaer]

clearly the SA are not working, let's wait to understand why...

[oliviermattelaer]

Ok Standalone issue is fixed and I have matched the version of madgraph5 directory to the one of master. This is ready to be merge. @valassi can you approve the merge?

[valassi]

Hi @oliviermattelaer thanks a lot, good patch. I made a few minor changes, I will let the CI run and approve and merge if it works. The change I made is that I continue to add those three files counters.cc, ompnumthreads.cc and fbridge_common.inc only in .mad directories, as they are not needed in the standalone case. In the python I did it in a "barbarian" way, you can probably do it better, but otherwise it works. I would have been able to do it better if the self.in_madevent_mode had been defined earlier, but I tested and it only seems to appear towards finalize(), so I did the changes in finalize. I also regenerated all processes (where as you intended two of those files become links in P* to SubProcesses). Ah and I also updated to merge with #811 which I had merged in the meantime.

[valassi]

Hi @oliviermattelaer for me thi sis good to go, But rather than merge myself, I ask you to review my changes and then decide if they are ok or you want to do better. Thanks!

[valassi]

@oliviermattelaer It seems I cannot set you (the author) as also reviewer. But let me know in case of issues, or just click yoursle on merge, Thanks

[valassi]

The commit to review is this one https://github.com/madgraph5/madgraph4gpu/pull/813/commits/7168a363d910df3391992aa8934b6c84c7747288

[oliviermattelaer]

I would say that the clean way to do that is to put an appropriate class hierarchy within output.py. Such that we have one class for standalone and one class for madevent (that one will inherit from standalone obviously).

Therefore I would propose the following actions: 1) discard your change in this branch 2) merge this branch 3) I'm going to create another branch where I change the class structure proposed above and implement the fact that those files are only needed for madevent.

What do you think?

[valassi]

Hi @oliviermattelaer thanks. Ok I see your point.

Like in many other cases, I think you have a better long term solution, what you propose for the long term makes sense.

And like in many other cases, we have a number of hacks and patches (many of these blame them on me) to get things working "in the meantime".

My general approach is, lets keep in mind the long term, but in the short term lets try to get this release out ASAP. In other words, lets have short term fixes and keep in mind that they need a cleanup.

In this specific case, there are two options:

• A) go your way, remove my patch: the disadvantage and thing to clean up later is that there are unnecessary files in the .sa directories of generated code (which were NOT there before your patch), by developing your better approach
• B) go my way, keep my patch: the disadvantage and thing to clean up later is to remove my ugly patch and develop your better approach (which needs to be done for option A as well!), but at least we do not introduce new files in .SA which are not needed and were not there before.

So I would go for my ugly patch option B. In any case we need to open an issue to develop your better approach. But I would do that on top of my option B, rather than on top of your option A. But you decide :-)

Thanks Andrea

PS Voila I opened issue #814 to work on what you suggest @oliviermattelaer

[oliviermattelaer]

I do not want to go to B and you do not want to go to A. Since we both agree on what the solution should be, I will implement it in this branch.

[oliviermattelaer]

So I did change the class structure and done it in a way such that they are no API change for the MG5aMC. But since you implement the test on the old API for MG5aMC, the madevent test are obviously not working since the test "instruct" the code to not be in madevent mode (even when in madevent mode).

So I will try to change the test now and hope to fix them.

[oliviermattelaer]

@valassi, do I have your go ahead for this merge after my changes? Or do you have reservation on the way I have done stuff? (or ...)

Thanks,

Olivier

[valassi]

sorry @oliviermattelaer I had no time to look again, I will try this afternoon

[oliviermattelaer]

Thanks, I have done the merge.

Olivier