Issue in the generation of the metadata of the fktables

[scarlehoff]

When pineko generates an fktable coming from the copnvolution of a grid and an eko, it can happen that the theory metadata gets saved in a json-incompatible format.

This is generally not a problem (since this metadata is never read in practice), however, the fonll module now looks at this key which raises an error at load
This PR fixes the error when reading: #163 the json data.
However we should ideally fix the problem at creation time, but I'm not sure where it is (honestly, I'm not even sure it is pineko and not something happening when the string goes through pineappl)

[alecandido]

I'm not sure if this is a problem only with FONLL combined FK tables, but this seems fishy:

pineko/src/pineko/fonll.py

Line 135 in d75ebe1

combined_fk.set_key_value("theory", str(theorycard))

I would have expected json.dumps(), not str().

[scarlehoff]

Yes, I'm pretty sure this one is wrong.

>>> str({"test": "oh, no"})
"{'test': 'oh, no'}"
>>> json.dumps({"test": "oh, no"})
'{"test": "oh, no"}'

However it must happen somewhere else (searching for either str( or set_key_value I could not find it) since this one is used after the load (so the bug has already happened). That's why I'm not sure it is pineko itself.

[alecandido]

In pinefarm there is this:
https://github.com/NNPDF/pinefarm/blob/39abf325d3ab8ec227f7c296935e55f966184434/src/pinefarm/tools.py#L212-L214
But it propagates whatever you already have (starting from JSON, you will obtain JSON...).

I also audited the rest of pinefarm, and it doesn't seem to have any chance of introducing something like that.

And yadism is correctly using json.dumps():
https://github.com/NNPDF/yadism/blob/bde1c764669f23e78898da06e11a6185ad098736/src/yadbox/export.py#L90

However, since you applied it only to the FONLL part, could it be that this has been done in some of the untracked scripts?
Not sure, it would be helpful to know which are the corrupted grids...

[scarlehoff]

since you applied it only to the FONLL part, could it be that this has been done in some of the untracked scripts?

Damn! My bad. I wrote it in the PR but didn't copy it here:

I was curious and checked that yadism actually writes this using json.dumps so the error was, in theory, impossible.

Indeed, the error is generated by pineko but I don't understand how and why it happens only for some datasets and not others.
If you are curious, HERACOMB_SIGMARED_B in theory 701 has this problem.

So it happens during FK generation in pineko. I only noticed it for FONLL because only the FONLL fktables are read by pineko after being dumpled (to do the combination) but it might happen also for others of course.

As you say, it might be some difference in the grid that a untracked script added and which propagates.

[alecandido]

You could run a scan, with a simple Python or shell script (just use jq '.' and check for the error code).

However, while fixing the error is for sure relevant, I'm not so sure if we could still attribute it in any way to Pineko...
(that is, this could now be an issue of some untracked script, and not really a Pineko one)

[scarlehoff]

I'm not so sure if we could still attribute it in any way to Pineko

Me neither. How do the theories get propagated from the grid to the fktable?

[alecandido]

pineko/src/pineko/evolve.py

Lines 261 to 283 in d75ebe1

	fktable = grid.evolve(
	ekompatibility.pineappl_layout(operators),
	xir * xir * mur2_grid,
	alphas_values,
	"evol",
	order_mask=order_mask,
	xi=(xir, xif),
	)
	rich.print(f"Optimizing for {assumptions}")
	fktable.optimize(assumptions)
	fktable.set_key_value("eko_version", operators.metadata.version)
	fktable.set_key_value("pineko_version", version.__version__)
	# compare before/after
	comparison = None
	if comparison_pdf is not None:
	comparison = comparator.compare(
	grid, fktable, max_as, max_al, comparison_pdf, xir, xif
	)
	fktable.set_key_value("results_fk", comparison.to_string())
	fktable.set_key_value("results_fk_pdfset", comparison_pdf)
	# write
	fktable.write_lz4(str(fktable_path))
	return grid, fktable, comparison

[alecandido]

Essentially is grid.evolve() doing the job.

https://github.com/NNPDF/pineappl/blob/778b7819059e963cfe74e042028683a42db79725/pineappl/src/grid.rs#L2062
https://github.com/NNPDF/pineappl/blob/778b7819059e963cfe74e042028683a42db79725/pineappl/src/grid.rs#L2098-L2101
https://github.com/NNPDF/pineappl/blob/778b7819059e963cfe74e042028683a42db79725/pineappl/src/fk_table.rs#L429

But it is a .clone() in Rust, so it's just copying memory from one side to another. I see nothing that could go wrong there...

[scarlehoff]

There's no direct reference to the theory there, should I assume it is happening in pineappl then?

Ok, then let me check later whether the problem is already present when arriving to that function and whether it appears immediately afterwards.

[alecandido]

grid.evolve() is already generating the fktable that is just later manipulated (but without touching that key). In the Rust code, `self.more_members contains the KV database, and the theory is stored in there (that's the thing that is cloned).