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

Hi,

> a) As far as the use of 'change process' for NLO reweighting is concerned, it
> seems both powerful but also quite tricky and not fully explained on the wiki
> when used in the NLO context.
> If this is intended to remain a 'hidden' functionality for now, I suggest that
> whenever a 'change process' is issued for NLO reweighting, a message warns the
> user that this is an alpha functionality that requires expertise.
> Anyway, many of the issues reported in my previous mail have been fixed over
> our skype session, so as far as 'change process' at NLO goes, things are fine
> like this, with maybe the message above if you like it too.

Will keep it like that, simple redefinition works. And this options is fully fonctional for LO type of reweighting.

> b) The 'change_rwgt_dir' option didn't seem to work.

Actually, I left that unchecked on purpose, In the idea to do it if someone (=CMS) want to have such mode. But since you point it here. I have taken the time to make it work.
Pretty sure no one will use it but ok.

> c) The loop initialization is maybe not ideal so if you skip it is fine, but
> then make sure to call the subroutine
> CALL ML5_0_FORCE_STABILITY_CHECK(.True.)
> to make sure that you get a stability estimate even during the first calls
> which will be during initialization for which there is no stability test by
> default. The above would enforce it.

Done.
Not fully sure where I have to put such line.
So please check (and if this is not correct please indicate me where you want such line)

> d) It is important not to use check_sa.f as being both the fortran driver
> *and* the f2py wrapper. This should be a separate file, say 'f2py_wrapper.f'
> that you use to make the matrix<n>.so.

Well I do not see the point here. check_sa is suppose to be a working template to present one way to call the loop computation nothing more right? So what is the point.

> e) Also, it would be nice to add the file 'f2py_wrapper.f' mentioned above to
> the default MadLoop standalone output. If you don't want to do it, then let me
> know when you'll have this file and I'll do it myself, no problem.

ok I put the file, but then you will need to modify the makefile since now it is pointless to have a file that you can not compile...

> f) In the MadLoop makefile there is a new rule which seems to be designed to
> make a static library for just one Pxxx process, i.e.

Here the best is that you perform the modification that you want directly.
It will be more efficient and less error prone. Especially since you have to change it anyway because of the above file.

> That's all, sorry for being such a picky reviewer; I promise to accept the
> branch right away once the above points are discussed/addressed ;) !

Thanks to you.

Cheers,

Olivier