-
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
Reimplement hera datasets #2175
base: master
Are you sure you want to change the base?
Conversation
import yaml | ||
|
||
@dataclass | ||
class commondata: |
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.
If the filters share similar or even identical parts, you can consider to place it in the filter_utils
folder and store common parts there. For ex:
from nnpdf_data.filter_utils.hera import commondata
hi @peterkrack on top of the checks, make sure to change the process_type to one of these https://github.com/NNPDF/nnpdf/blob/master/validphys2/src/validphys/process_options.py (there should be some DIS types available) and change the kinematic files to use the variables listed there (which to 1st approximation means changing k1, k2, to x,Q or the other way around). You might need to change Q to Q2 in the kinematic files as well. |
…rtainties from systematics, moved class commondata to utils module.
…o make the tests pass.
Hi @peterkrack what is the status of this? |
Hi @peterkrack, sorry to bother you again, what is the status of this? |
Hi @peterkrack could you include in this PR data-theory comparisons and xq kinematic coverage plots? Thanks |
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 @peterkrack are you available this week to do the changes? Otherwise I'll do them so that we can merge this that has been stale for a few weeks now...
Plots with k2binsX in NC datasets are not a perfect match yet. |
Setting kinematics_override back to dis_sqrt_scale fixed the issue with the plots. |
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.
HERA reimplementation is ready for review.
@peterkrack are there any data theory comparisons for all the distributions implemented here? if not, can you please produce a report? |
HERA_CC_318GEV https://vp.nnpdf.science/UpTc36imQAW1_rQGtoarhA== |
x: | ||
description: Variable x | ||
label: x | ||
units: '' | ||
k2: | ||
description: Variable k2 | ||
label: k2 | ||
Q2: | ||
description: Variable Q2 | ||
label: Q2 | ||
units: '' | ||
k3: | ||
description: Variable k3 | ||
label: k3 | ||
y: | ||
description: Variable y | ||
label: y |
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.
please update the description, label and units here.
Once the above comment is taken care of, this can be merged. |
Added rawdata and filter.py script for HERA dataset.
Checks of reimplemented data are still missing.