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

Hi Olivier,

> Hi Valentin,
>
> Thanks so much
>
> > 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.
>
> Sure will do.
>
> > Anyway, for the present version the fix seems fine but I don't see any test
> related to this.
>
>
> In fact I just kept the previous test, written by Johan who were checking
> those points and edit the
> the tests according to the new ordering. So I think that this is fine.
>

Sorry, I had overlooked the modification of the export_v4_test.
Yes, I'm looking forward to seeing such kind of tests ported to the IOTest framework as it will make their updates so much more handy.
The kind of tests I was referring to where more generic, e.g. not comparing the output vs an hardcoded one but really scanning through the helas wavefunction lists and checking that the order is consistent (all mothers precede their daughters).
But, indeed, I guess the existing 'hardcoded' test is good enough for now.

Cheers,

> Cheers,
>
> Olivier
>
> On May 14, 2013, at 6:23 AM, Valentin Hirschi <email address hidden>
> wrote:
>
> > Review: Needs Fixing
> >
> > 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,
> >
> >
> > --
> > https://code.launchpad.net/~maddevelopers/madgraph5/1.5.10/+merge/163092
> > You proposed lp:~maddevelopers/madgraph5/1.5.10 for merging.