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

Revision history for this message
Eleni Vryonidou (evryonidou) wrote :

Hi Olivier,

when I was looking at this a few weeks ago I realised that in BinothLHA.f there is this line:

 if ((dabs(avgPoleRes(1))+dabs(avgPoleRes(2))).ne.0d0) then
               cpol = .not.((((PoleDiff(1)+PoleDiff(2))/
     $ (dabs(avgPoleRes(1))+dabs(avgPoleRes(2)))) .lt.
     $ tolerance*10d0).or.(mod(ret_code,10).eq.7))

and
               cpol = .not.((PoleDiff(1)+PoleDiff(2).lt.tolerance*10d0)
     $ .or.(mod(ret_code,10).eq.7))

I believe Collier corresponds to .eq.7 and this is why the test seems to pass, even if the numbers are not agreeing. E.g. I see things like:

 ---- POLES CANCELLED ----
  COEFFICIENT DOUBLE POLE:
        MadFKS: -5.1118724822382218E-008 OLP: 0.0000000000000000
  COEFFICIENT SINGLE POLE:
        MadFKS: -5.4796083693610551E-008 OLP: -9.2617064392846240E-008

Maybe someone can confirm why that line was added there. It’s not the source of the zero numbers, but it’s weird that this was set up like that to allow the test to pass.

Cheers,

Eleni

> On 20 May 2020, at 20:38, Olivier Mattelaer <email address hidden> wrote:
>
> Hi,
>
> Let's try to avoid any confusion here:
> 1) we do not have any issue with ninja.
> - we can still install it
> - when install it works
>
> 2) For some unknow reason, my version of 3.0.2 version was configured as if ninja was not installed in my machine, in that case collier is the next OLP provider and is used as default.
>
> 3) We have a bug related to collier that does not return the correct pole (I'm therefore worried about the other output like the finite part of the loop). This bug has been spotted by Eleni and Gauthier and independently by me (thanks to point 2 above)
>
>
>> We must keep COLLIER as it is crucial for loop-induced efficiency.
> I could not reproduce seg-faults in standalone (and neither did Richard I
> believe).
>
> Being fast is useless if we are wrong. Like this, this sounds like a bad bug.
> If you are sure that this is a MadFKS related bug, then we can forbid the usage of COLLIER only in that case. But it would be nice that you take a real look at this. I prefer to be slow and correct than fast and wrong.
>
> Cheers,
>
> Olivier
> --
> https://code.launchpad.net/~maddevelopers/mg5amcnlo/3.0.2/+merge/381317
> Your team MadDevelopers is subscribed to branch lp:~maddevelopers/mg5amcnlo/3.0.2.

« Back to merge proposal