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

Revision history for this message
Olivier Mattelaer (olivier-mattelaer) wrote :

Hi Valentin,

> I like the default reweight card much better this way.
> However, I would change this:

Not really happy for those proposal.
So I have made another version which should be clearer (and stress that both NLO_VT and NLO_basic are both NLO accurate).

> which is nice but there is still a problem with your dependence on lhapdf with
> the exact NLO reweighting. You must really check for this at the beginning and
> take action correspondingly.

Ok this is handle in a nicer way and the pdf set are automatically downloaded now.

> What about doing a trial import with this:
>
> subprocess.call("python -c 'import module_or_modules_in_question'")
>
> allowing you to nicely handle a crash there? I know this seems clunky but I
> think it is ok and really much better than a python crash. I mean MG5aMC
> crashing with no message whatsoever is really scary for the user and looks
> bad; we should really avoid it at all cost if possible!

I can do that, but this is obviously very limited since all C++/fortran error at run time will not be catched (like Lhapdf do not find the pdfset/... even if python finds it/...)
So I think that the nicer method would be to run the full reweighting in a separate subprocess.
I will do that.

> You have to call this function:
>
> MadLoopInitialize.init_MadLoop in madevent_interface.py
>
> yourself. But it really is a one-liner, so it should really be easy.
> It is best to initialize it first so that you have a trustworthy accuracy
> estimate for the first couple of events you reweight.
>

I tried but it seems ... not working:
root: Error while executing make in /Users/omatt/Documents/eclipse/nlo_reweighting/PROCNLO_loop_sm_85/rw_mevirt/SubProcesses/P1_csx_epve
root: Failed at running the process in /Users/omatt/Documents/eclipse/nlo_reweighting/PROCNLO_loop_sm_85/rw_mevirt/SubProcesses/P1_csx_epve.

> I don't think so (or I don't understand what you meant). The ratio of loop ME
> for the original and new hypothesis can for sure depend on the renormalization
> scale, so it is not enough that the numerator and denominator have the same
> scale, it must *also* be the same scale as used in the original event I think

DONE

Thanks a lot,

Olivier

« Back to merge proposal