`ThPredictionsResult` should not use C++

[siranipour]

At the moment when computing theory predictions we make calls to C++

nnpdf/validphys2/src/validphys/results.py

Line 131 in 70ca11d

th_predictions = ThPredictions(loaded_pdf, loaded_data)

afaict this can be replaced in favour of validphys.convolution. In particular the stats no longer need to be done using C++ too. However, before fixing this I just want to make sure of the following:

When we have

nnpdf/validphys2/src/validphys/results.py

Line 108 in 70ca11d

return self._std_error

Is this equivalent to:

np.std(self._rawdata, axis=1)

because at the moment, (for NMC for example)

np.allclose(self.std_error, np.std(self._rawdata, axis=1)
# False

cc @scarrazza

[scarlehoff]

Hi @siranipour, are you working on this atm? Otherwise I'll try to do this together with #1420

[siranipour]

I tried, saw it was a bit more involved than an afternoons work and gave up. I'm happy to take a look properly if you like?

[scarlehoff]

No. Don't worry. Between 1420 and the pineappl/eko business I'll have to deal with all of this anyway.

But no worries I'll need you to review the PR which I anticipate that will also be more than one afternoon :P

[scarlehoff]

There is a problem for this, currently libNNPDF allows to do the convolutions without cuts.
Instead, validphys does not

nnpdf/validphys2/src/validphys/convolution.py

Line 116 in 2fc5586

raise PredictionsRequireCutsError(

As a result some runcard cannot work: https://github.com/NNPDF/nnpdf/blob/master/validphys2/examples/data_theory_comparison.yaml

I don't know why MW added that error, @Zaharid @siranipour, any ideas? Does libNNPDF always apply errors internally (if so, that runcard would be wrong?)

Other than that, this issue should be easy to deal with _{(famous last words)}

[Zaharid]

The problem is that some data points are not present in some fktables and are in practice always cut out. Libnnpdf would always return zero for convolutions of these. I couldn’t find a particularly elegant way of making the fkparser do something sensible like eg returning nans and hence cuts were required.

…

On 8 Oct 2021, at 12:36, Juan M. Cruz-Martinez ***@***.***> wrote:  There is a problem for this, currently libNNPDF allows to do the convolutions without cuts. Instead, validphys does not https://github.com/NNPDF/nnpdf/blob/2fc5586489e33c0a1c63f8ac93f2db390fe998b7/validphys2/src/validphys/convolution.py#L116 As a result some runcard cannot work: https://github.com/NNPDF/nnpdf/blob/master/validphys2/examples/data_theory_comparison.yaml I don't know why MW added that error, @Zaharid @siranipour, any ideas? Does libNNPDF always apply errors internally (if so, that runcard would be wrong?) Other than that, this issue should be easy to deal with (famous last words) — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe. Triage notifications on the go with GitHub Mobile for iOS or Android.

[scarlehoff]

Is this (in practice) equivalent to "internal"?

[Zaharid]

Equivalent in what regard? The requirement is to have cuts that are compatible with the data in the fktables of which the internal cuts are an example. So we either need that or find a way to deal with the missing points.

…

On 8 Oct 2021, at 13:13, Juan M. Cruz-Martinez ***@***.***> wrote  Is this (in practice) equivalent to "internal"? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe. Triage notifications on the go with GitHub Mobile for iOS or Android.

[scarlehoff]

You said

The problem is that some data points are not present in some fktables and are in practice always cut out.

and

Libnnpdf would always return zero for convolutions of these

I wonder whether settings the cuts to internal will have the same practical effect (in this case that the points that would be set to 0 by libNNPDF are dropped). If that were to be the case then having cuts as a dependent of the convolutions is fine to me.

In any case, the runcard is wrong in that cuts are being applied by virtue of some points not existing at all in the prediction.

[Zaharid]

I still don't see what you are are asking (as in what are you trying to do now), but it might be that I got confused myself with this a few times. In any case the discussion from when I understood it best is here #1141

[scarlehoff]

Right now libNNPDF convolutions do not require any cuts, instead the python ones do.

You said the cuts are always there (in the form of 0s) so the only solution I see is: The intrinsic cuts (I guess internal) should be required to run any runcard containing convolutions (libNNPDF or python).

[scarlehoff]

Indeed, my solution is basically what MW suggest in there. I'll do that. The current way of doing it is far from the best and if someone wants to recover some specific behaviour in that case they can implement it anew in python.

I'll add a warning explaining the problem for people trying to use old runcards.

Edit: In summary, I don't think reimplementing past design decisions is necessary once we accept to drop 3.1 compatibility.