Re-enable use_t0 flag.

[scarlehoff]

See #1489 (comment)

It is in principle a trivial change that should not do anything, adding @andreab1997 in case there was a reason why we wanted to eliminate use_t0 that I've forgotten.

[andreab1997]

See #1489 (comment)

It is in principle a trivial change that should not do anything, adding @andreab1997 in case there was a reason why we wanted to eliminate use_t0 that I've forgotten.

The point is that we wanted to have two separate options for using t0 in the sampling and/or in the fitting. This is why now the use_t0 flag can be used both under the fitting namespace and under the sampling namespace. If we want to enable again a "global" use_t0 flag we need to understand what is its meaning I think (i.e. if we use it, does it mean that we want to use t0 prescription in both sampling and fitting? or just in the fitting?)

[scarlehoff]

It is for things like vp reports where fitting/sampling don't apply.

I'm actually wondering whether it shouldn't be true by default instead....

[andreab1997]

It is for things like vp reports where fitting/sampling don't apply.

I'm actually wondering whether it shouldn't be true by default instead....

If it is only for reports yes, most likely it should be true by default

[Zaharid]

I'd say this should work as it has been documented (i.e. as in this PR), and the other change may have inadvertently corrupted results, e.g. anyone who has been computing chi2 vs experimenal chi2 recently (@RoyStegeman, @J-M-Moore ?).

Similarly changing the default is going to be very disruptive for existing runcards, so please don't.

I'd say use_t0 should mean which chi2 are the various tables/plot/fit computing, and the huge complexity of the covmats should be handled by a separated mechanism (or ideally eliminated).

[Zaharid]

I'd prefer if the default was as it was.

[Zaharid]

fwwi this is the documented behavior

https://docs.nnpdf.science/figuresofmerit/index.html#avoiding-bias-t0-method

[scarlehoff]

Similarly changing the default is going to be very disruptive for existing runcards, so please don't.
fwwi this is the documented behavior

Uh, didn't realize (since for the fits it defaults to True). I'll roll it back (and I'll change the use_in_fitting to False as well, since n3fit sets it as True anyway.

and the other change may have inadvertently corrupted results [...] anyone who has been computing chi2 vs experimenal chi2 recently

I think only in the case:

t0pdfset: someset
use_t0: false

and only if the covmat used doesn't go through config. So i hope not

[github-actions]

Greetings from your nice fit 🤖 !
I have good news for you, I just finished my tasks:

• Fit Name: NNBOT-46d863b92-2022-09-15
• Fit Report: https://vp.nnpdf.science/qx6sv2JaRGKM6ao22pN9vw==
• Fit Data: https://data.nnpdf.science/fits/NNBOT-46d863b92-2022-09-15.tar.gz

Check the report carefully, and please buy me a ☕ , or better, a GPU 😉!

[scarlehoff]

Ok, it works (compare with last report) https://vp.nnpdf.science/IzbRlZ4IQ8qbZddbo0B8dw==/
Please review and merge.

At some point we should really tackle #1025 in a systematic way. The current reference is way too old.

[Zaharid]

I don't believe the current behaviour is what we want either. If I say use_t0: False, use_t0_sampling: True I want the chi2 to be the experimental one (and the sampling flag to be ignored for that purpose). Also I'd think use_t0 should be the same as use_t0_fitting.

[Zaharid]

Maybe a better way to go about this, both making it nicer and maintaining compatibility is to remove produce_t0_set and specifically the possibility of it being None, and instead have a bunch of explict_node based on use_t0 and the other flags.

[scarlehoff]

I don't believe the current behaviour is what we want either.

Something something about not discussing policy #1528 (comment)

In all seriousness, better to have the default behavior being "use t0 for the fit" given that the runcards for NNPDF4.0 are already "out there" and don't include use_t0 in any form or way. If you don't want to use t0 for the fit then you have to say it explicitly.

[Zaharid]

Didn't understand that last comment (neither the serious part nor the quibble). Indeed I think the defaults should be as they were for 4.0, even if the whole thing is a bit convoluted. That means that n3fit implicitly defaults to using t0 (but we may want to make sure you can disable that, e.g. by having the script set use_t0: True in some namespace that can be overriden by the runcard) and vp implicitly defaults to computing the experimental chi2 (regardless of whether a t0 pdf is specified and mos certainly regardless of any flags with "sampling" in them).

Things that we should avoid are that the hundreds of runcards like https://vp.nnpdf.science/RGfy_dVZQa6xYT72MFW_7w==/ suddenly start failing screaming about some t0 pdf, or that the vp reports for fits with funny sampling configuration start computing the t0 chi2 inadvertently.

[scarlehoff]

The first version behavior is equal to the current version.
The only difference is that I put fitting to False since it is already set to true by n3fit.

To your specific point:

If I say use_t0: False, use_t0_sampling: True I want the chi2 to be the experimental one (and the sampling flag to be ignored for that purpose)

In this case I would say you want the use_t0_sampling: True. Since it is ambiguous I can compromise on throwing an exception if such an ambiguous input is given.

Things that we should avoid are that the hundreds of runcards like https://vp.nnpdf.science/RGfy_dVZQa6xYT72MFW_7w==/ suddenly start failing screaming about some t0 pdf,

This is what this PR fixes (wouldn't that runcard fail now because of nocuts?)

or that the vp reports for fits with funny sampling configuration start computing the t0 chi2 inadvertently.

The vp report should only look at use_t0 and the fit should only look at fitting/sampling.

[Zaharid]

The fundamental problem is that the thing that could be None to indicate that t0 was not used only makes sense if you can assume that t0 is either used or not in the whole namespace, which is not the case if we want to also have individual control over the sampling covmat. Hence why believe the design should be changed to explicit nodes (or see if we actually need the new functionality).

[scarlehoff]

we actually need the new functionality

We needed because we are sampling the replicas with python now. We didn't need it before because the replicas came from nnpdfcpp

believe the design should be changed to explicit nodes

Feel free to change it, but I think we should quickly fix the current buggy behaviour in master.

[Zaharid]

I don't see how the new behaviour is not buggy.

[scarlehoff]

Then please fix it so it stops being buggy.

[scarlehoff]

The current version will raise a ConfigError when it finds something weird in the input (like @Zaharid example in #1599 (review)) while working with n3fit runcards (where use_t0 and use_t0_fitting are True by default) and validphys runcards (where use_t0 is False by default and use_t0_fitting and use_t0_sampling should not be used... and if they are used the ConfigError will be raised).

Please check if the current version satisfies all the interested parties as I would like to fix the current master (which is bugged for sure). If not please post a very specific example of something broken so I don't need to try out different combinations in order to guess what the problem is.

For new designs please open a new issue where it can be discussed.

[scarlehoff]

@Zaharid any news here?

[Zaharid]

Closed in favour of #1626