Skip to content

Reimplement ATLAS WPWM 7TeV 36PB#2223

Merged
scarlehoff merged 14 commits into
masterfrom
reimplement_ATLAS_WPWM_7TEV_36PB
Jan 21, 2025
Merged

Reimplement ATLAS WPWM 7TeV 36PB#2223
scarlehoff merged 14 commits into
masterfrom
reimplement_ATLAS_WPWM_7TEV_36PB

Conversation

@ecole41
Copy link
Copy Markdown
Collaborator

@ecole41 ecole41 commented Nov 22, 2024

Functions haves been added to produce the data central, kinematic and uncertainties yaml files for this dataset.

Old vs New Data Report

https://vp.nnpdf.science/6-mHiiJJTcC5Mf86u4NJ2g==/

Compatabiliy Check

[In]:from validphys.api import API
import numpy as np
 
inp1 = {"dataset_input": {"dataset": "ATLAS_WPWM_7TEV_36PB_ETA"}, "theoryid": 40_000_000, "use_cuts": "internal", "t0pdfset": "NNPDF40_nnlo_as_01180", "use_t0": True}
inp2 = {"dataset_input": {"dataset": "ATLASWRAP36PB", "variant": "legacy"}, "theoryid": 40_000_000, "use_cuts": "internal", "t0pdfset": "NNPDF40_nnlo_as_01180", "use_t0": True}

covmat1 = API.covmat_from_systematics(**inp1)
covmat2 = API.covmat_from_systematics(**inp2)
 
t0_covmat1 = API.t0_covmat_from_systematics(**inp1)
t0_covmat2 = API.t0_covmat_from_systematics(**inp2)
 
result = np.all(np.isclose(covmat1, covmat2))
result_2 = np.all(np.isclose(t0_covmat1, t0_covmat2))

print('covmat', result)
print('t0_covmat', result_2)

[Out]:LHAPDF 6.5.4 loading [/Users/ellacole/miniconda3/envs/nnpdf_dev/share/LHAPDF/NNPDF40_nnlo_as_01180/NNPDF40_nnlo_as_01180_0000.dat](https://file+.vscode-resource.vscode-cdn.net/Users/ellacole/miniconda3/envs/nnpdf_dev/share/LHAPDF/NNPDF40_nnlo_as_01180/NNPDF40_nnlo_as_01180_0000.dat)
NNPDF40_nnlo_as_01180 PDF set, member #0, version 1; LHAPDF ID = 331100
covmat True
t0_covmat True

@scarlehoff
Copy link
Copy Markdown
Member

I think it is better to finish the ones that are already started and then move to the more weird datasets ^^U

When the data don't match, try to take the ratio and check whether the difference is just normalization. Some of the old datasets were normalized for bin size / total cross section / other stuff.

@scarlehoff
Copy link
Copy Markdown
Member

scarlehoff commented Nov 27, 2024

Hi @ecole41 I've had a look and the tables from https://www.hepdata.net/record/ins928289 match the data up to a factor of 1.0187 for all bins.

I have checked only Table 1 (which is the Z) against https://github.com/NNPDF/nnpdf/blob/master/nnpdf_data/nnpdf_data/commondata/ATLAS_Z0_7TEV_36PB/data_legacy_ETA.yaml
(there's also a factor 1000 of fb vs pb)

In cases like this, when there's one single factor for all bins, your best bet is to look at the old buildmaster and... indeed...

const double lcorr = 1.0187; // correction factor due to luminosity upgrade

file: kinematics.yaml
theory:
conversion_factor: 1.0
conversion_factor: 1.0187 #is this needed?
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure if I should include this conversion factor here

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looking at commondataparser.py this is valid input! So I would keep it.

@ecole41 ecole41 changed the title [WIP] Reimplement ATLAS WPWM 7TeV 36PB Reimplement ATLAS WPWM 7TeV 36PB Dec 18, 2024
@ecole41 ecole41 changed the title Reimplement ATLAS WPWM 7TeV 36PB [WIP] Reimplement ATLAS WPWM 7TeV 36PB Dec 18, 2024
@jacoterh
Copy link
Copy Markdown
Collaborator

@ecole41 if you could please redo the report including conversion_factor = 1.0187 it's easier to compare theory and data. Thanks.

@ecole41
Copy link
Copy Markdown
Collaborator Author

ecole41 commented Dec 18, 2024

@ecole41 if you could please redo the report including conversion_factor = 1.0187 it's easier to compare theory and data. Thanks.

The report was made with the conversion factor in the metadata already, so it the old and new should hopefully match. I will remove the comment now

Comment thread nnpdf_data/nnpdf_data/commondata/ATLAS_WPWM_7TEV_36PB/metadata.yaml Outdated
Comment thread nnpdf_data/nnpdf_data/commondata/ATLAS_WPWM_7TEV_36PB/metadata.yaml Outdated
@jacoterh
Copy link
Copy Markdown
Collaborator

@ecole41 if you could please redo the report including conversion_factor = 1.0187 it's easier to compare theory and data. Thanks.

The report was made with the conversion factor in the metadata already, so it the old and new should hopefully match. I will remove the comment now

Ah interesting, is there a reason why the theory is consistently higher than the data then? I see it's the same in the old implementation, so it's technically not part of this PR, but I got curious.

@ecole41
Copy link
Copy Markdown
Collaborator Author

ecole41 commented Dec 19, 2024

@ecole41 if you could please redo the report including conversion_factor = 1.0187 it's easier to compare theory and data. Thanks.

The report was made with the conversion factor in the metadata already, so it the old and new should hopefully match. I will remove the comment now

Ah interesting, is there a reason why the theory is consistently higher than the data then? I see it's the same in the old implementation, so it's technically not part of this PR, but I got curious.

Are we sure that the conversion factor should be included in the metadata? I have already included it when producing the data.yaml file so maybe it isn't needed in the metadata also? Then maybe the theory would be more consistent with the data

@jacoterh
Copy link
Copy Markdown
Collaborator

onversion factor should be included in the metadata? I have already included it when producing the data.yaml file so maybe it isn't needed in the metadat

Maybe, the best way here is to simply try what the theory vs data comparison looks like without including this factor, i.e. setting it to one.

@ecole41
Copy link
Copy Markdown
Collaborator Author

ecole41 commented Dec 19, 2024

onversion factor should be included in the metadata? I have already included it when producing the data.yaml file so maybe it isn't needed in the metadat

Maybe, the best way here is to simply try what the theory vs data comparison looks like without including this factor, i.e. setting it to one.

Here is with conversion_factor: 1.0187: https://vp.nnpdf.science/6-mHiiJJTcC5Mf86u4NJ2g==/

Here is with conversion_factor: 1.0: https://vp.nnpdf.science/4JZ3wcPYTDOGHtDyu70pjg==/

There is a difference but I am not sure which one is correct. I would guess to keep the conversion factor equal to 1 as this has already been included in the data.yaml? But that might be wrong

@ecole41 ecole41 changed the title [WIP] Reimplement ATLAS WPWM 7TeV 36PB Reimplement ATLAS WPWM 7TeV 36PB Jan 8, 2025
@scarlehoff
Copy link
Copy Markdown
Member

Hi @ecole41 this includes the correction we talked about on Wednesday and it can be merged right? (same for the Z 36PB PR)

Copy link
Copy Markdown
Member

@scarlehoff scarlehoff left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a bit confused about this one. It seems the experimental uncertainties are much smaller. I think in the comparison you were always using the legacy variant, as you were using the old name instead of the new name.

In order to keep compatibility, whenever the new name is being used, it defaults to the legacy variant.

In any case, I believe the difference is coming from atlaslumi10 which in the new implementation is always a factor of 10 smaller. Could you check whether this is a bug (or whether the previous dataset was bugged and the luminosity was a factor of 10 too big).

Also, just in case, could you check also the datasets you already merged (if any). Since you were checking with the legacy names some small bugs could've passed unnoticed.

label: k1
abs_eta:
description: Absolute pseudo-rapidity of the W boson
label: abs_eta
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
label: abs_eta
label: r"|$\eta|$"

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can see where the issue is, I have implemented the 3.5% lumi uncertainty incorrectly and inserted it as '0.35'. I will change this now and check the others also

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To produce reports using the new data (not the legacy), is it a case of just changing the dataset to the new dataset name? I think I remember that it was not working when I did this but I can try this again

Copy link
Copy Markdown
Member

@scarlehoff scarlehoff Jan 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. You need to use the new name (and then variant legacy will take the old data).

The reason why using the old name automatically selects the legacy is so that we can reproduce the old runcard that were using the old names (the mapping is in datasets_names.yml).

Let me know if it doesn't work, there might be some bug.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When I change the name to the new dataset, I get this error:

/Users/ellacole/codes/nnpdf/nnpdfgit/nnpdf/validphys2/src/validphys/deltachi2.py:123: SyntaxWarning: invalid escape sequence '\c'
  label=f"{pdf.label} - $\chi^2_{0}$={total_chi2_data.central_result:.0f}",
[ERROR]: Bad configuration encountered:
Dataset ATLAS_WPWM_7TEV_36PB not found
Instead of 'ATLAS_WPWM_7TEV_36PB', did you mean one of the following?
 - ATLAS_WP_JET_8TEV_PT
 - ATLAS_WM_JET_8TEV_PT
 - ATLAS_2JET_7TEV_R06

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The complete name is ATLAS_WPWM_7TEV_36PB_ETA (<setname>_<observable_name>)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you, I have redone the compatibility checks and the old vs new report : https://vp.nnpdf.science/5lqgkuPJTleEVVvwvAZC_Q==/ using the new dataset name, and after changing the ATLASLUMI uncertainty to 3.5%. They look to match and the uncertainties match the old legacy uncertainties also

Comment thread nnpdf_data/nnpdf_data/commondata/ATLAS_WPWM_7TEV_36PB/metadata.yaml Outdated
@scarlehoff scarlehoff merged commit fda3d4f into master Jan 21, 2025
@scarlehoff scarlehoff deleted the reimplement_ATLAS_WPWM_7TEV_36PB branch January 21, 2025 17:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants