-
Notifications
You must be signed in to change notification settings - Fork 7
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Tevatron inclusive DY #2185
Tevatron inclusive DY #2185
Conversation
Thanks for this @jacoterh!
Now, you can use this function: nnpdf/nnpdf_data/nnpdf_data/filter_utils/utils.py Lines 402 to 423 in bf8c890
and call it as follows in the filter:
Also, same as in the other PRs, could you check that the datasets are loaded properly and that the (t0) covmats are the same? from validphys.api import API
import numpy as np
inp1 = {"dataset_input": {"dataset": f"{new_implementation}"}, "theoryid": 40_000_000, "use_cuts": "internal", "t0pdfset": "NNPDF40_nnlo_as_01180", "use_t0": True}
inp2 = {"dataset_input": {"dataset": f"{old_implementation}", "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)
np.all(np.isclose(covmat1, covmat2))
np.all(np.isclose(t0_covmat1, t0_covmat2)) |
url: '' | ||
version: -1 | ||
url: https://www.hepdata.net/record/ins856131?version=1&table=Table%202 | ||
version: 1 | ||
implemented_observables: | ||
- observable_name: ZRAP | ||
observable: | ||
description: Drell-Yan Rapidity Distribution | ||
label: CDF $Z$ rapidity (new) | ||
units: '' | ||
process_type: EWK_RAP |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @jacoterh
Remember to put as process_type:
one of the processes that you will find in this file: https://github.com/NNPDF/nnpdf/blob/master/validphys2/src/validphys/process_options.py (at the bottom, either DY_Z_Y or DY_W_ETA)
and also put kinematic_override: identity
.
This is to avoid going through some old code that we are trying to remove.
When you do that you will see that that file defines which kinematic variables are allowed, sometimes it is necessary to change from Q -> Q^2 or pT -> pT^2 (the kinematic info in the end is the same, but we are trying to standardize it a bit more)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, done!
Thanks @Radonirinaunimi! The numbers have been formatted correctly now. I have also performed the check you proposed using the snippet you provided, see for instance here . If you run this script, you'll find that the 3rd bin in D0_Z0_1P96TEV differs from the legacy implementation for the reason I mentioned in the beginning of this PR. Some questions:
|
The current failure is because it is The previous one probably related to some change in digits looking at the logs. |
validphys reports:
The last disagreement in D0_Z0_1P96TEV_ZRAP can be explained as follows. The systematic uncertainty in the 3rd bin appears symmetric already on hepdata, while the legacy implementation seems to have symmetrised plus and minus uncertainties in the past here. This also affects the central value. |
Thanks Jaco. One last thing, could you check that also the experimental covmat for the 3 datasets together doesn't change for non-diagonal entries? (you can also e.g. just check that the experimental chi2 for all 3 considered together is unchanged) (this will be important for the singletop data as well, to check that possible experimental correlations across datasets are also captured) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @jacoterh
Thanks for this. All seems fine, the only thing, could you bump the version
and change the version_comment
in the metadata.yml
files?
RE the failing test, you might need to regenerate pseudodata_test_fit_n3fit_240916
. Since it is only the pseudodata that is relevant, you can just do:
vp-fitrename pseudodata_test_fit_n3fit_240916 pseudodata_test_fit_n3fit_241112
cp pseudodata_test_fit_n3fit_241112/filter.yaml pseudodata_test_fit_n3fit_241112.yaml
And then just regenerate the data for the 10 replicas, no need to wait until the fit is done.
@scarlehoff I have tried to follow your steps but I am a bit lost. This is what I did.
and launched the fit with
This gives me a fitted replica, but then what do I commit to make sure the tests don't fail anymore? |
Upload the new fit to the server (so that it is available) amd in conftest change the name of the fit. If you then run pytest for that test it should work. |
Thanks, I ran the fit on 20 replicas, evolved them, but now postfit filters out 90% of them: And if it's just the pseudo data that we need, we don't even care right? |
Yes, you don't even need to finish the fit. That's why if you use fit rename you get the previous one and just take need the new data csvs |
Let me know whether I can merge this. |
6fd5029
to
e0edfd8
Compare
For future reference, this is the (hopefully) final validphys comparison: https://vp.nnpdf.science/PceSQWJaSLCrCp7wHzrsWg==/ If the tests pass, this branch should be ready for merging |
This PR implements filter functions for the Tevatron inclusive DY datasets:
General comment: small overall differences are due to formatting. For example: 1.2999999999999998 instead of 1.3. Is there any function that was used in the past to uniform the way floats were formatted?