Fixing the grouping and the thcovmat flag

[andreab1997]

The grouping used in the thcovmat fit is broken because groups_dataset_inputs_loaded_cd_with_cuts does not use group_dataset_inputs_by_fitting_group and therefore is not grouped as the thcovmat (even if the groupings are similar so sometimes it happens to be correct by chance).

Moreover, the thcovmat group is used also when both use_thcovmat_in_sampling and use_thcovmat_in_fitting are false.

[andreab1997]

@scarlehoff as you can see I have only changed group_dataset_inputs_by_metadata for group_dataset_inputs_by_fitting_group in the collect of groups_dataset_inputs_loaded_cd_with_cuts. This was enough to solve our problem. However there a lot of other places in which group_dataset_inputs_by_metadata is used. I wonder if would be better to change them all. What do you think?

[scarlehoff]

I wonder if would be better to change them all.

We only want to order by fitting group when we are fitting. I'm not sure this is a good solution actually, I think the "metadata group" should be equal to the "fitting group" during a fit. Is there a reason why they should be different?

[andreab1997]

I wonder if would be better to change them all.

We only want to order by fitting group when we are fitting. I'm not sure this is a good solution actually, I think the "metadata group" should be equal to the "fitting group" during a fit. Is there a reason why they should be different?

The point is that by default metadata group groups by experiment. So if we use fitting_group for the covmat and metadata_group for the groups_dataset_inputs_loaded_cd_with_cuts, we will have in general different ordering

[scarlehoff]

The point is that by default metadata group groups by experiment.

Then I think the right thing is for metadata group to default (in the case of a fit) to the fitting group.

[andreab1997]

The point is that by default metadata group groups by experiment.

Then I think the right thing is for metadata group to default (in the case of a fit) to the fitting group.

nnpdf/validphys2/src/validphys/config.py

Line 1581 in 05602d6

def load_default_data_grouping(self, spec):

This is the point in which the default data_grouping is stated. Do we want to change this?

[scarlehoff]

Yes. Indeed, I can even see that @wilsonmr left a comment many moons ago predicting this day would come :P

[Zaharid]

FWIW while the while "data keyword" grouping system was great in both allowing complex new uses of the code and maintaining compatibility, I think the resulting trade off in complexity is not favorable to us anymore, in particular since we don't have to support an experiments declaration anymore. Simplifying that would be very useful work attacking the base of this problem.

[andreab1997]

Yes. Indeed, I can even see that @wilsonmr left a comment many moons ago predicting this day would come :P

Do you like the new solution more?

[github-actions]

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

• Fit Name: NNBOT-600d2fe11-2022-08-25
• Fit Report: https://vp.nnpdf.science/c9fNw04rTh2reMDWgOE6PQ==
• Fit Data: https://data.nnpdf.science/fits/NNBOT-600d2fe11-2022-08-25.tar.gz

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