-
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
New Common Datasets #1813
New Common Datasets #1813
Conversation
@scarlehoff, during our meeting (w/ @cschwan and @peterkrack) this afternoon we were wondering what would happen if the different variants have different number of datapoints? Especially given that |
Remember this happened for the infamous CDF Z measurement, where they removed the last point. |
If I remember correctly it should not possible to have variants with different number of datapoints. The variants are for different treatment of the uncertainties and that kind of thing. If the data is different then it is a different dataset. |
That would trigger either an update with a new commit and a comment in the |
Ok, agreed! |
b2e9195
to
e64d092
Compare
0c9b2bd
to
902e9bb
Compare
Co-authored-by: Juan M. Cruz-Martinez <juacrumar@lairen.eu>
902e9bb
to
cf55efb
Compare
Co-authored-by: Felix Hekhorn <felixhekhorn@users.noreply.github.com>
Automatic port old commondata to the new format
I'm going to merge this to master. I'll leave the scripts I've used in the buildmaster folder for future reference, they might be useful. |
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.
It would be indeed good to have this merged!
This branch/PR will collect all datasets which are fully implemented in the new commondata format.
When you finish a dataset (or group of datasets) please create a PR pointing to this branch. Then the PR will be tested by either @enocera or myself, ensuring that not only the data and uncertainties are correct but also that the validphys plotting works, all necessary variables defined, etc.
Before opening a PR please ensure that:
buildmaster/dataset_names.yml
withNEW_NAME: OLD_NAME
.plotting
,kinematics
andtheory
sections are filled with all the necessary information.nnpdf31_process
matches whatever was thennpdf31_process
in the old version.legacy
(or similar) variant in order to reproduce the old results.(I've update this list with more points that might be important to remember!)
In principle you should be able to use the new commondata reader: #1678 to check by yourself that everything is working.
The definition of the new commondata is documented in #1708. Please let me know if anything is missing.
In practical terms, you don't need to branch out of this branch. When you are done a think that a dataset is finished, you can either open a PR only with that dataset of with a collection of them and I will deal with the merge.
Please don't change files outside of the datasets that you are implementing unless absolutely necessary.