[WIP] Test implementation of new commondata format

[enocera]

This PR addresses #1436.
The proposed layout is as follows. For each data set, there is a folder called DATASET_NAME. Each folder contains:

• a metadata.yaml file;
• a filter.py file.

The filter.py script reads the metadata.yaml file and generates no less than three .json files:

• data.json, for the central values of the data;
• kinematics.json for the kinematics of the data;
• uncertainties.json for the uncertainties of the data.

There can be as many variants as needed for each of these three files. Variants are specified in the metdata.yaml.

TO DO

• include the missing filter.py
• have a look at the layout and at its prototypical realisation;
• write a validphys reader that takes as input the .json files in lieu of the 'DATAandSYSTYPE` files. (Add vp parser for new commondata format #1511)

[scarlehoff]

One thing missing from the list (I only realised now that I started with the parser) would be an automatic installation.

I guess to first approximation these should be part of the validphys package, right? Since the fit needs validphys anyway.

(it's not important for the parser, I can hardcode the folder for now)

[scarlehoff]

@enocera
One question, is it the idea that the variants are now selected in a way similar to how we select the kfactors? For instance one can have as input:

dataset_input:
  - dataset: "NMCPD"
  - variant: "dw"

and if no variant is selected it defauls to the standard one.

[enocera]

@scarlehoff Do you mean in a vp runcard?

[scarlehoff]

Yes. Instead of having to write NMCPD_dw

[enocera]

Then yes, I think that we want to have something similar in spirit to, but more general than, the current sys key. cc @Zaharid

[enocera]

Or, as you say, the cfac key.

[scarlehoff]

Great, I like this idea. I'll implement it in this spirit. The idea is then that "sys" will disappear?

[enocera]

Yes, definitely. The problem with sys is that it only controls the way in which systematic uncertainties are correlated or not. The variant is something more general (and cleaner) which is specified by a set of data.json and uncertainty.json files. This means that you can have different central values and different uncertainties (correlated in whatever way) for each dataset (without the need of renaming the dataset). In my opinion, this is useful to have nuclear variants, but also, e.g., EW variants (and of course also more manageable debugged variants).

[scarlehoff]

Do we want to have a CommonDataSpec and a CommonData as two separate classes or can we unify them as CommonDataSpec that will have all the attributes of both? This one is for @Zaharid I guess

[scarlehoff]

And for @enocera, this is a perfect and total substitution, right? So I can just scrap all the "old" commondata stuff as I find it in the code? No need to keep legacy wrappers like I have to do for the fktables.

[enocera]

And for @enocera, this is a perfect and total substitution, right? So I can just scrap all the "old" commondata stuff as I find it in the code? No need to keep legacy wrappers like I have to do for the fktables.

I'd answer yes because the idea (my idea) is to rework all the datasets in the new format.

[scarlehoff]

Are these null expected? What should be returned if an user asks for the "minimums array", just the same thing as the avg?

[enocera]

I placed a null because this is a case in which bin edges are not provided by the experimentalists; I'd say that if null, then min=avg.

[scarlehoff]

I see, thanks.
I also I think there might have a bug in the script that outputs the json because the kinematics_max and kinematics_min are missing a comma in front.

Also, I think [{"x":null},{"Q2":null},{"y":null}] could just be a single dictionary: {"x":null, "Q2":null, "y":null} (I'm using the null as example because the arrays are big, but the same applies.

[scarlehoff]

Suggested change

	tables:
	tables:

[Zaharid]

Do we want to have a CommonDataSpec and a CommonData as two separate classes or can we unify them as CommonDataSpec that will have all the attributes of both? This one is for @Zaharid I guess

I like the separation between header and object and also would like the parsing stage of vp to be as fast as possible, so rather prefer the arrangement where we have two classes.

[scarlehoff]

Suggested change

	variants:
	- nuclear_uncertainty_choice:
	deweighted: uncertainties_dw.json
	shifted: uncertainties_dw.json
	variants:
	deweighted: uncertainties_dw.json
	shifted: uncertainties_dw.json

I think it is better to have just one level of variance here, otherwise the fit runcards might become too complicated.
Then we might want to have metadata in the variance type of course.

[Zaharid]

Can variants change things other than the uncertainties?

[enocera]

That is a very good question. The answer is yes, in the future. Example: datasets for which EW corrns have not been subtracted, in the case in which these are instead in the theory. So maybe we should list both the data.json, uncertainty.json and kinematics.json files for each variant?

[scarlehoff]

I like the idea of having a default selection and then allowing the variant to change a number of things.

So:

variants
  deweighted:
    uncertainties: uncertainties_dw.json
  ew:
     data: ew_subtracted.json

etc

[scarlehoff]

I like the separation between header and object and also would like the parsing stage of vp to be as fast as possible, so rather prefer the arrangement where we have two classes.

This can be done with one single class as well. In any case, I already started putting all in one for the toy model, separating it in two should be easy.

Instead what I'm separating is KinematicSpec and UncertaintiesSpec (which right now were somewhere in libNNPDF I guess)

[Zaharid]

The one class we need to have is the one representing precisely whatever the user wrote in the runcard. As @RoyStegeman can tell, not having that e.g. for positivity is a source of of pain.

[scarlehoff]

That has nothing to do with this, the kinematics or uncertainties are not written in the runcard but are part of the metadata of the commondata and that needs to be held by the CommonDataSpec.

[Zaharid]

One big annoyance with json for scientific applications is that it cannot handle NaN or infinity. I wonder how relevant is that in here. For one we could parse the same files with yaml and write e.g. .inf for infinity.

[enocera]

One big annoyance with json for scientific applications is that it cannot handle NaN or infinity. I wonder how relevant is that in here. For one we could parse the same files with yaml and write e.g. .inf for infinity.

@Zaharid Are you wondering how relevant .json vs .yaml is or whether NaN or inf are relevant? If you're wondering about the first, I'm in favour of moving from .json to .yaml.

[Zaharid]

I guess both.

[enocera]

So, I don't think that NaN or inf can appear as sensible values of data central values or uncertainties or kinematic bins. Moving to yaml is perhaps advisable anyway - incidentally I used .json instead of .yaml only because this is what was originally suggested.

[scarlehoff]

Hi @enocera, I don't understand the format of the uncertainties:

{'sys_1': ['ADD', 'CORR', <numbers>}

What should I parse from here. Are the numbers following ADD and CORR the list of ADD and CORR concatenated together? Wouldn't it be better to have maybe {sys_1: ([list of add], [list of corr])}?

Or even directly a list of dictionaries

[{`ADD`: [numbers], 'CORR': [NUMBERS]},]

and then the number of systematic is just the number of dictionaries in the list (so no need to keep track with the sys_x name).

[enocera]

So:

• sys_1 is the 1-st sytematic uncertainty; we can have as many sys as we need, and I thought to enumerate them like sys_1, sys_2, ... , sys_i, ..., sys_n, where n is the number of systematic uncertainties;
• sys_1 in this case is additive (ADD) and correlated (CORR) at the same time; the nature (additive vs multiplicative) and correlation of each uncertainty should be specified for each sys_i (and can be different for different is);
• <numbers> correspond to sys_i over the j data points.

[scarlehoff]

Ah! Ok. Then I guess we should have some kind of metadata (thinking about yaml in this example):

sys_1:
   mode: ["ADD", "CORR"]
   values: [array of ndata numbers of points]

[enocera]

Yes, definitely. This is what I was thinking about after Zahari suggested to abandon .json in favour of .yaml.

[Radonirinaunimi]

There was also the suggestion to add the information on what is being fitted here. This might be worth adding already?

[Zaharid]

As far as I understand, you still manually download the tables from HepData? This could be automatized in order to avoid this manual work. HepData has a hepdata-cli/-api but this does not seem to easily provide us with what we need. In addition, I don't think we'd rely on a package that is not maintained. Instead, below is a simple sample function that can retrieves the tables for us.

We have discussed doing this several times and it would indeed be nice if we could have it in a pull request form. A key requirement is the ability to check that the current version of the tables matches the latest version in hepdata.

[Radonirinaunimi]

We have discussed doing this several times and it would indeed be nice if we could have it in a pull request form. A key requirement is the ability to check that the current version of the tables matches the latest version in hepdata.

I agree that this is definitely something that we want. This could be easily implemented and something I can take care of.

[Radonirinaunimi]

I now have a module that reads the hepdata URL from metadata.yaml and download the corresponding tables. It does so by first checking if the corresponding dataset has already been downloaded, and if yes checks if the version matches the latest version on hepdata.

This can be used to perform what you, @Zaharid, mentioned above. But I just want to make sure that I understand exactly what you have in mind. What I have been playing with is a github action that--using the module--checks every now and then hepdata if a new version of a dataset is available. And if so, it opens a PR with the new version and keeps pushing commits whenever new versions are available until the PR is merged. Is this somehow in line with what you think?

[scarlehoff]

Hi @Radonirinaunimi, in this branch there are some utilities that might be useful (while the dataset implementation is now outdated). Do you want to keep these tools?

Otherwise we can close this PR.

[Radonirinaunimi]

Hi @Radonirinaunimi, in this branch there are some utilities that might be useful (while the dataset implementation is now outdated). Do you want to keep these tools?

Otherwise we can close this PR.

I think it'd be better to close this PR, and if needed I'd implement these utilities in another PR. Especially since after having re-implemented datasets, I'd approach these utilities in a different way now.

[scarlehoff]

ok, thanks!