Re-implementation of the LHCb Collider DY Datasets

[Radonirinaunimi]

The following PR implements the LHCb Collider DY datasets in the new commondata format. The table summarizes the status of the implementation and provides general comments on the comparisons w.r.t. the old implementation. The dataset names in the new tables are the new ones and the mapping to the old ones is tracked in dataset_names.yml.

Dataset Name	Comparison vs. Old	General Comments	Status
LHCB_Z0_7TEV_DIELECTRON_Y	☑️		☑️
LHCB_Z0_8TEV_DIELECTRON_Y	☑️		☑️
LHCB_Z0_13TEV_Y-DIELECTRON	☑️		☑️
LHCB_Z0_13TEV_Y-DIMUON	☑️		☑️
LHCB_Z0_7TEV_MUON_Y	☑️		☑️
LHCB_DY_7TEV_MUON_Y	☑️		☑️
LHCB_Z0_8TEV_MUON_Y	☑️		☑️
LHCB_DY_8TEV_MUON_Y	☑️		☑️
LHCB_WPWM_7TEV_MUON_Y	☑️		☑️
LHCB_WPWM_8TEV_MUON_Y	☑️		☑️

Final Report: https://vp.nnpdf.science/4ub8cAYxTa2xPP6oM6X4_w==

[scarlehoff]

I know this is just a draft, but since I was already reviewing some other dataset PR I thought it was better to add some quick comments so that they are automatically applied to the other datasets that are going to be implemented!

[Radonirinaunimi]

Thanks for the quick feedback @scarlehoff. I was indeed about to address some of the points you raised above after reviewing everything and tested with #1678. I haven't done so yet but I already addressed most of your comments in cc85bb9.

Maybe as a general comment, it would be useful to have #1708 up to date, to include kinematic_override for example? (This was the reason why I forgot about it although we discussed it)

[scarlehoff]

kinematic_override might not be necessary (at the moment for the ones that don't use it I asked @t7phy to just put identity for the time being until I decide what to do with the xq plot) but when doing a re-implementation of the old commondata the whole plotting file need to be ported over.
Another option is of course modifying the kinematics so the override is not necessary but I think for the re-implementation it is better to leave them as close to the original as possible.

[cschwan]

Commit eb45404 adds the dataset that I've been working on, which still has lots of unfilled fields (marked with TODO). If someone could give this a review I'd be grateful!

[Radonirinaunimi]

Thanks @cschwan! I've tried to address the TODO's below. A minor comment, you should also add the mapping to the old names in dataset_names.yml.

[scarlehoff]

Hi @Radonirinaunimi, many thanks for this.

I've had a first go at this set of datasets, I've added the possibility of using extra_labels with label_based figure_by to #1678 so now the datasets in this branch would work.

The tests I'm doing for every datasets are three:

1. Check that the central data is exactly the same.
2. Check that computing the chi2 agrees exactly (meaning that the covmat is read and created equally)
3. And finally doing a report.

The test 1 is passed by every dataset.
Test 2 is passed for the following pairs:

"LHCBWZMU7TEV", "LHCB_DY_7TEV_MUON_Y"
"LHCBWZMU8TEV", "LHCB_DY_8TEV_MUON_Y"
"LHCBZEE2FB_40", "LHCB_Z0_8TEV_DIELECTRON_Y"
"LHCB_Z_13TEV_DIELECTRON", "LHCB_Z0_13TEV_Y-DIELECTRON"
"LHCB_Z_13TEV_DIMUON", "LHCB_Z0_13TEV_Y-DIMUON"

while for these

"LHCBZMU7TEV", "LHCB_Z0_7TEV_MUON_Y",
"LHCBZMU8TEV", "LHCB_Z0_8TEV_MUON_Y",
"LHCBWMU7TEV", "LHCB_WPWM_7TEV_MUON_Y",
"LHCBWMU8TEV", "LHCB_WPWM_8TEV_MUON_Y",
"LHCBZ940PB", "LHCB_Z0_7TEV_DIELECTRON_Y"

the value of the chi2 is created differently. Please have a look at them!
I've had a look at the covmat and even the diagonal is different.
In particular, for LHCB_Z0_7TEV_DIELECTRON_Y there's something going on because the new covmat has values like 10^6. There might be an order-of-magnitude problem in some uncertainties?

3. I've done the report for the 5 dataset that fully agree so that you can have a look and check that everything is correct. I cannot see any obvious problems but of course you've been implementing them so might know better!
   The report is ordered by pairs (so first/second, third/four, etc)

https://vp.nnpdf.science/vY3OvQMqTxybygGVncMkvQ==

Edit: If there is no obvious fix to the 5 that produce a different covmat I would be happy with separating the 5 that are 100% ok and merging them by themselves.

[Radonirinaunimi]

@scarlehoff, if you run the checks for the remaining datasets everything should now be Ok. I wasn't able to produce the report which includes all the datasets myself due to the pandoc stacksize error (that I need to find a solution to).

There were basically two main problems. For LHCB_Z0_7TEV_DIELECTRON_Y, the issue was that the normalization uncertainty was off by $10^3$. For the rest, since these are separation of the NC and CC components from the same measurement, I previously only dumped the uncertainties corresponding to the process-bins. Now, all the correlated uncertainties are dumped and that fixed the issue.

[scarlehoff]

Thank you very much! I find agreement for all of them and the plots generally look the same as before afaics

Here's a report with all datasets. Please check that everything looks as you expected. By that I mean, plotting options, axis scale and all that.
(I've used the QCD cfactor for the prediction, and NRM for the ones that needed it as well).

https://vp.nnpdf.science/4ub8cAYxTa2xPP6oM6X4_w==

Once you confirm this is ok, this is ready to merge. However, after looking at #1785 I've realized the old-new mapping syntax might need to change a bit. If it does, I'll add the appropriate change it here (and update the docs) so nothing to do from your side, but just to mention it!

(pinging @enocera in case he wants to have a second look or want to add anything to any of it before merging)

[enocera]

@Radonirinaunimi Thanks for your work. @scarlehoff I'd like to have a second look at the implementation before merging, if you don't mind.

[Radonirinaunimi]

As far as I am concerned, this is basically ready. So once @enocera approves (thanks for also having a look) we can merge this.

[scarlehoff]

I've repeated the checks with the final version fo this branch:

1. Central value are the same
2. The covariance matrix are the same
3. The computed chi2 is the same
4. The t0 chi2 is the same (so multiplicative and additive are equivalent in the new and old implementation)

The report in this comment #1826 (comment) is unchanged since @Radonirinaunimi has not updated any metadata.yaml files (only the uncertainties) in his last commits.

So this one is perfectly ready to go from my side.

[Radonirinaunimi]

Thanks a lot for these additional checks @scarlehoff! This is completely done from my side as well.