MadGraph5_aMC@NLO

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

bias_function
Merge into 2.5.6

Revision history for this message

Stefano Frixione (stefano-frixione) wrote on 2017-08-07:

Hi Olivier,
some comments on your comments.

> 1) I do not like very much to use "event_norm" to trigger the bias. The
> main reason is that this flag is used to control how the weight of
> the event should be written. How would I set the normalization of the
> event to "Sum" in presence of bias?
well, tastes aside (I personally like "bias") this option has a rather
direct bearing on the nature of the weights written in the LHEF, and
this includes their "norm". Furthermore, "bias" (or equivalent)
conveys immediately the message that these, at variance with the
other weights produced out of the integration, are not unweighted.

> Do we really need a flag in the run_card? Can't we have the default bias
> function to be "1"?
I strongly oppose this. The current choice may indeed be redundant,
but it is the only safe one. The possibility of screwing with this
stuff is significant, so we do need an extra layer of protection.

What can be done (no strong feelings here) is to have a check that,
if the bias function is identically equal to one, the warning printouts
are removed from the output.

> Like this, this will creates problem with the re-weighting/systematics
> module (but trivial to fix) since they are all using that variable to
> know how to handle the weights.
I think we shouldn't do reverse engineering here: it's a new option,
so the systematics module must cope with it.

> 2) the HTML/printout results, only display the un-physical number (with
> a very nice warning). Should not we automatically display the
> correct result? I guess that this is fine like this since we want to
> have this only for very advanced user.
Well, I thought that the number printed out resulted from the integration,
not from the sum of event weights (since the former is more precise).
That being the case, the currect choice has no alternative, I'm afraid
(which is OK with me).

> 3) So For the moment, the only way to get the physical cross-section is
> trough the systematics printout. However for the "example" bias
......
> It seems to be a numerical precision issue, since if I change the bias
......
> So maybe we should a comment about this "problem" in cuts.f (looks like
> we are much more sensitive than at LO)
Well, this is expected, isn't it? By biasing, one shouldn't obtain
a particularly reliable number for inclusive rates, but smoother
high-pt tails. A comment, if any, should stress only this fact.

> 4) After fixing systematics.py in order to assume that bias
>
> So this indicates a problem in the way the weight is written in the file.
....
Note: in my tests, I've only looked at distributions. Although I've
mostly concentrated on the central weights, a quick looks at results
for non central scale values didn't show anything strange.
I'm using standard analyses as provided in ./MCatNLO subdirectories.

Cheers, Stefano.

> Did you change that format? did you add one entry?
> This is important since this will also forbid the re-weighting actually.
> (We can skype about this if you want to).
>
>
>
>
>
>
> --
> https://code.launchpad.net/~maddevelopers/mg5amcnlo/bias_function/+merge/328265
> You are requested to review the proposed merge of lp:~maddevelopers/mg5amcnlo/bias_function into lp:~mg5core1/mg5amcnlo/2.5.6.
>

Hi Olivier,
some comments on your comments.

> 1) I do not like very much to use "event_norm" to trigger the bias. The
>    main reason is that this flag is used to control how the weight of
>    the event should be written. How would I set the normalization of the
>    event to "Sum" in presence of bias?
well, tastes aside (I personally like "bias") this option has a rather 
direct bearing on the nature of the weights written in the LHEF, and
this includes their "norm". Furthermore, "bias" (or equivalent)
conveys immediately the message that these, at variance with the
other weights produced out of the integration, are not unweighted.

> Do we really need a flag in the run_card? Can't we have the default bias 
> function to be "1"?
I strongly oppose this. The current choice may indeed be redundant,
but it is the only safe one. The possibility of screwing with this
stuff is significant, so we do need an extra layer of protection.

What can be done (no strong feelings here) is to have a check that,
if the bias function is identically equal to one, the warning printouts
are removed from the output.

> Like this, this will creates problem with the re-weighting/systematics 
> module (but trivial to fix)  since they are all using that variable to 
> know how to handle the weights.
I think we shouldn't do reverse engineering here: it's a new option,
so the systematics module must cope with it.

> 2) the HTML/printout results, only display the un-physical number (with
>    a very nice warning).  Should not we automatically display the
>    correct result? I guess that this is fine like this since we want to
>    have this only for very advanced user.
Well, I thought that the number printed out resulted from the integration,
not from the sum of event weights (since the former is more precise).
That being the case, the currect choice has no alternative, I'm afraid
(which is OK with me).

> 3) So For the moment, the only way to get the physical cross-section is
>    trough the systematics printout. However for the "example" bias 
......
> It seems to be a numerical precision issue, since if I change the bias 
......
> So maybe we should a comment about this "problem" in cuts.f (looks like 
> we are much more sensitive than at LO)
Well, this is expected, isn't it? By biasing, one shouldn't obtain
a particularly reliable number for inclusive rates, but smoother
high-pt tails. A comment, if any, should stress only this fact.

> 4) After fixing systematics.py in order to assume that bias 
>
> So this indicates a problem in the way the weight is written in the file.
....
Note: in my tests, I've only looked at distributions. Although I've
mostly concentrated on the central weights, a quick looks at results
for non central scale values didn't show anything strange.
I'm using standard analyses as provided in ./MCatNLO subdirectories.

Cheers, Stefano.

> Did you change that format? did you add one entry?
> This is important since this will also forbid the re-weighting actually.
> (We can skype about this if you want to).
>
>
>
>
>
>
> -- 
> https://code.launchpad.net/~maddevelopers/mg5amcnlo/bias_function/+merge/328265
> You are requested to review the proposed merge of lp:~maddevelopers/mg5amcnlo/bias_function into lp:~mg5core1/mg5amcnlo/2.5.6.
>

« Back to merge proposal