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

Hi Johan,

>I have redone the implementation in a minimal manner. This way, none of our previous >functionality is affected, which I believe is the most safe way to do it (especially >considering
>the *enormous* amount of time I have spent on getting the fermion flow correct for all cases).I
>certainly don't dare touch that part of the code unless it's absolutely necessary.

At the end this is quite similar to what I did.
- one modification to need_hermitian (both).
- one modification to need_charge_conjugate (both).
So I don't understand your comment above.

The main differences between those two versions are
1) that I didn't veto the Conjugate on identical majorana (which creates the trouble with the tests that you mention)
2) that you factorize everything in one function (see below)
3) that you treat 4 fermion case, (Excellent that you did)

But I really consider my implementation much more simpler to understand. (Off course I'm biased)

I would strongly suggest to split majorana_conjugates
in two separate functions. One for the wavefunctions and one for the amplitudes.
the number of if in this case checking in which case we are (and some of them quite implicit)
makes the code un-readable.

Please also split the long if. This will make the code more readable.
some if test in fact two completely different things
1) if both particle are majorana
2) if the flow is correct
make two if is maybe longer (and with additional else) but much more clear.

Another problem, but with wich we can life with, is that they the structure is now quite un-efficient.

in get_call_key:
1) it check if it need the hermitian
--> one call the majorana_conjugates
2) it ask for the conjugate index
--> call to get_conjugate_index
    This one check first if if need the hermitian conjugate
    --> second call to majorana_conjugates
    Then call itself majorana_conjugates
    --> third call to majorana_conjugates

Then at a point we will call get_used_lorentz
which will make a call to get_conjugate_index
So two additional call to majorana_conjugates

So five calls to this functions this is a bit pointless.
We can:
1) store the data (forbids the static method but fine if you split the routine in two as suggested above) such that the data is compute only once.
2) Refactorize the code to remove the need_conjugate routine.
(We will pass from 5 to 2 call). -- I prefer this one since the modif are very small at the end--
3) keep like that (As I said this is not critical for the moment at least)

Finally, your test is about MG4 Model, which are in fact insensitive to this problem in mg5.
(Since the call for such routine are not implemented) Are they a good reason for not using UFO syntax in your tests?

Cheers,

Olivier