Code review comment for lp:~maddevelopers/mg5amcnlo/3.0.2-alpha0

Revision history for this message
marco zaro (marco-zaro) wrote :

Hi Olivier,
thanks for your comments

> On 14 Oct 2021, at 16:26, Olivier Mattelaer <email address hidden> wrote:
>
> Review: Approve
>
> This is very good and basically ready to be merged.
>
> Some minor point.
> 1) !z! works ... should not be this forbidden?
yes, but it should do nothing
> 2) can we use tagged photon with QCD correction only?
> -- In the commit I did, I have added unittest that crash related to the above command, you can remove those test if those are fine—
In principle the syntax should work, giving identical results with the case without tagging
> 3) 2!a! was making the code to crash but !2a! was working fine. I have updated the parser to support both.
I was not aware of the fact that one can use the numbers to specify multiple times the same particle
> 4) I have spotted a typo in banner.py (see the comment within the diff for that)
I have fixed it, please have a look
> 5) I have loose track of all the change in cuts.f but did not spot anything obviously wrong. If you are not confident enough about that part maybe I should go deeper and track that we did not "loose" any previous cut but sounds like it.
>
Let us see what Rik says here…

Cheers,

Marco

> Thanks a lot and sorry for being slow on this review
>
> Diff comments:
>
>>
>> === modified file 'madgraph/various/banner.py'
>> --- madgraph/various/banner.py 2021-07-29 07:14:14 +0000
>> +++ madgraph/various/banner.py 2021-10-04 14:52:23 +0000
>> @@ -4552,11 +4553,20 @@
>> if proc_characteristic['ninitial'] == 1:
>> #remove all cut
>> self.remove_all_cut()
>> +
>> + # check for tagged photons
>> + tagged_particles = set()
>>
>> # Check if need matching
>> min_particle = 99
>> max_particle = 0
>> for proc in proc_def:
>> + for leg in proc['legs']:
>> + if leg['is_tagged']:
>> + tagged_particles.add(leg['id'])
>> +
>> + if 22 in tagged_particles:
>> + self['gamma_is_j'] = False
>> min_particle = min(len(proc['legs']), min_particle)
>> max_particle = max(len(proc['legs']), max_particle)
>
> The min_particle/max_particle should not be in the if ""22" block but still in the
> for proc in proc_def: loop
>
>> matching = False
>
>
> --
> https://code.launchpad.net/~maddevelopers/mg5amcnlo/3.0.2-alpha0/+merge/408723
> Your team MadDevelopers is subscribed to branch lp:~maddevelopers/mg5amcnlo/3.2.1.
>

« Back to merge proposal