Code review comment for lp:~maddevelopers/mg5amcnlo/1.5.10

Revision history for this message
Valentin Hirschi (valentin-hirschi) wrote :

Hi Olivier,

I'll do my best for this review. Since there is nothing major, I went pretty fast on it but still noted the following:

a)

Depending on the c++ compiler, the include
"#include <iostream>"
is mandatory at the beginning of a C++ code in order to use the std:: namespace.
All cpp template have it, could you add it to the new txxxxx template routine too.

b)

This fermion flow flipping business is getting even worse with your necessary modifications. At some point, one will have to rethink that. One possibility would be for example to give up on the dynamical on-the-flight recycling optimisation and perform it only at the very end, after all helas_diagrams have been generated.

Anyway, for the present version the fix seems fine but I don't see any test related to this. Given that we know this part to be a very sensitive point of MG5, it would be worth to have the order of the wfs tested. Something like going through the list of wfs for various processes featuring majoranas and making sure that each wf finds its mothers in the wavefunction list placed before and having smaller nb_wf.

For example, when doing line 1060 to 1070 of helas_objects.py, you are not certain that the mothers of new_wf are before the index i in diagram_wavefunctions list or in previous diagrams in the wavefunctions list. I agree this should be true by construction, but one can never be too careful.

I looked superficially at the rest and it looks fine.

Cheers,

review: Needs Fixing

« Back to merge proposal