Fit with theory covmat with n3fit #1528

andreab1997 · 2022-02-22T16:53:21Z

This will make possible to run a NNPDF4.0 fit including theory_covariance matrix

Edit: it also modifies the replica generation (make_replica) to utilize the covmat (with theory errors and multiplicative uncertainties if the proper flags are utilized)

…p_covmat

n3fit/src/n3fit/scripts/n3fit_exec.py

scarlehoff

Could you also add one example runcard (and maybe a regression test for the invcovmat?)

Also -for @Zaharid - would you be against using .npy instead of .csv? I'm don't have a strong opinion on this but it would make things a bit faster and leaner (for a 4000x4000 matrix this is 382M vs 123M)

There is some conflict in the docs (I guess you modified the same document in this PR and also in the other one). Please fix that so that the merge is not blocked.

n3fit/src/n3fit/scripts/n3fit_exec.py

validphys2/src/validphys/config.py

validphys2/src/validphys/covmats.py

Zaharid · 2022-03-10T11:37:33Z

Also -for @Zaharid - would you be against using .npy instead of .csv? I'm don't have a strong opinion on this but it would make things a bit faster and leaner (for a 4000x4000 matrix this is 382M vs 123M)

Yes please, do that.

Zaharid · 2022-03-10T11:37:55Z

Or, compute the covmat dynamically, not sure if that is better.

Zaharid · 2022-03-10T11:49:53Z

We also had #1091 which turned out to be a bit more involved and was forgotten about. Also probably @alecandido convinced me that npy is reasonable enough for this kind of purposes. I guess parquet still has the advantage of giving you an richer index.

scarlehoff · 2022-03-10T11:54:28Z

As a note, I think having a csv in the regression tests is good. Having the changes in human readable form is nice (you can always parse them of course). But anyway, this would only be for the storing of the thcovmat.

Although maybe it makes sense to compute it on the fly indeed. @andreab1997 how long does setupfit take?

andreab1997 · 2022-03-10T12:07:54Z

As a note, I think having a csv in the regression tests is good. Having the changes in human readable form is nice (you can always parse them of course). But anyway, this would only be for the storing of the thcovmat.

Although maybe it makes sense to compute it on the fly indeed. @andreab1997 how long does setupfit take?

For the runcards I tried, it takes about 40 minutes against the 20 of a fit without theory covmat

scarlehoff · 2022-03-10T12:54:01Z

For the runcards I tried, it takes about 40 minutes against the 20 of a fit without theory covmat

Then I guess it's better to store it and load it.

andreab1997 · 2022-03-11T14:10:29Z

I believe that the only left change to apply is the one related to .csv vs .npy, right? Anyway, I have not understood the final decision, should we dump the thcovmat in csv or npy? (leaving as given that we do not want to compute it online)

PS: @scarlehoff there are still your requested changes blocking the merging but I took already care of them. How can I solve this problem?

scarlehoff · 2022-03-11T14:12:42Z

Anyway, I have not understood the final decision, should we dump the thcovmat in csv or npy?

Yes, please, using .npy will be faster and will use less memory!

How can I solve this problem?

You can't! I have to look through the changes!

alecandido · 2022-03-11T14:12:48Z

PS: @scarlehoff there are still your requested changes blocking the merging but I took already care of them. How can I solve this problem?

I guess the proper solution would be to re-request a review: if you solved them, you should allow your reviewer to check that the solution is satisfactory (and not breaking anything else...).

andreab1997 · 2022-03-11T14:18:15Z

PS: @scarlehoff there are still your requested changes blocking the merging but I took already care of them. How can I solve this problem?

I guess the proper solution would be to re-request a review: if you solved them, you should allow your reviewer to check that the solution is satisfactory (and not breaking anything else...).

ok done, thank you :)

andreab1997 · 2022-03-11T14:37:08Z

Anyway, I have not understood the final decision, should we dump the thcovmat in csv or npy?

Yes, please, using .npy will be faster and will use less memory!

How can I solve this problem?

You can't! I have to look through the changes!

I am sorry, I am having problems finding the place where the writing happens. Can you please suggest me the right place where to look?(I believed this should have been in produce_nnfit_theory_covmat but I cannot find the writing) @scarlehoff @Zaharid

scarlehoff · 2022-03-11T15:00:19Z

I'm going to guess the action that vp-setupfit loads has a @table decorator on top? Creating a new action that does the same but without the decorator (and with a numpy.save at the end) should do the trick.

Zaharid · 2022-04-07T16:06:34Z

@scarlehoff yes please.

andreab1997 · 2022-04-29T08:18:17Z

@scarlehoff yes please.

Can this be merged? @Zaharid

andreab1997 · 2022-05-30T15:14:43Z

@Zaharid I have solved the conflict. Can this be merged now?

…covmat_fit

scarrazza · 2022-06-27T11:30:03Z

@RoyStegeman could you please have a quick review of this PR so we can merge?

RoyStegeman · 2022-06-27T11:51:34Z

Sure

validphys2/examples/theory_covariance/Fit_with_theory_covmat.yml

n3fit/src/n3fit/scripts/vp_setupfit.py

n3fit/src/n3fit/scripts/n3fit_exec.py

validphys2/examples/theory_covariance/Fit_with_theory_covmat.yml

RoyStegeman · 2022-06-30T10:46:39Z

validphys2/src/validphys/covmats.py

+    norm_threshold=None,
+    dataset_inputs_t0_predictions,
+    loaded_theory_covmat,
+    ):


What Juan says

RoyStegeman · 2022-06-30T10:48:32Z

validphys2/src/validphys/covmats.py

+    use_weights_in_covmat=True,
+    norm_threshold=None,
+    dataset_inputs_t0_predictions,
+    ):


what Juan says (here and functions below)

validphys2/src/validphys/covmats.py

validphys2/src/validphys/pseudodata.py

validphys2/src/validphys/config.py

scarrazza · 2022-07-06T14:24:39Z

@andreab1997 could we merge this?

andreab1997 · 2022-07-06T14:34:59Z

@andreab1997 could we merge this?

For me yes. I don't know if @RoyStegeman agrees.

RoyStegeman · 2022-07-06T14:40:54Z

Sure please go ahead

andreab1997 · 2022-07-06T14:46:58Z

@andreab1997 could we merge this?

Actually give me till this evening, I have to fix a minor thing and then I will merge.

RoyStegeman · 2023-04-01T21:27:21Z

validphys2/src/validphys/config.py

+                if f == path:
+                    raise ValueError(
+                        "More than one theory_covmat file in folder tables"
+                    )


@andreab1997 this doesn't seem right?

The error is not as general as I originally thought. It probably does do what you intended, but I tried it with a custom covmat and got this error because I did not set use_scalevar_uncertainties to false, in which case this message does not make a lot of sense since there was only one covmat.

So I guess we just need to update the example here: https://github.com/NNPDF/nnpdf/blob/278310410398b36b95e45246992c8ed07db6d6a6/doc/sphinx/source/tutorials/general_th_covmat.rst? If that's correct, I can update the docs.

P.S. the check can be written in one line for example as

if set(paths)&set(files): raise ValueError

Ok yes I agree. I would say the check is checking what I wanted (even if it can be written better) but the error message is misleading.

I guess there is no situation in which both use_scalevar_uncertainties and use_user_uncertainties are true? If so, that should probably be checked separately by vp. Are there valid situations in which both are false?

For the moment I would say no but in principle yes (for example one can have a different source of theory uncertainties which are not user_uncertainties)

Sure, but here use_scalevar_uncertainties is just synonym for using a covmat that has been constructed from different theoryID's (regardless of the source of the theory uncertainty) with the instructions in the runcard, while user_uncertainties is an external covmat (not necessarily non-scalevar).

So for now I think use_scalevar_uncertainties can probably be deprecated as it is just the inverse of user_uncertainties?

If you agree, I can take care of the changes. Just you know this better than me, so it's good if you can confirm :)

Yes I agree. In my mind we could have a third option, like use_whatever_uncertainties, that uses a different prescription from the scalevar. However it is true that we do not have it now so, for the time being,use_scalevar_uncertainties is just not user_uncertainties. So yes, I agree :)

Let's leave hypothetical future features for hypothetical future people to work on ;)

Ok, I'll open a PR some point this week and ask for your review.

Fixing loaded_commondata_with_cuts import and add theory_covmat to ex…

3469368

…p_covmat

andreab1997 marked this pull request as draft February 22, 2022 16:53

Zaharid requested a review from scarlehoff February 24, 2022 12:08

andreab1997 added 3 commits February 24, 2022 18:37

Fixed loading of theory_covmat also for user provided covmat

80bba9d

Fixed theory_covmat flags

815a595

Fixed some doc

539f366

andreab1997 self-assigned this Mar 9, 2022

andreab1997 marked this pull request as ready for review March 9, 2022 14:22

scarlehoff reviewed Mar 10, 2022

View reviewed changes

n3fit/src/n3fit/scripts/n3fit_exec.py Outdated Show resolved Hide resolved

scarlehoff requested changes Mar 10, 2022

View reviewed changes

n3fit/src/n3fit/scripts/n3fit_exec.py Outdated Show resolved Hide resolved

validphys2/src/validphys/config.py Outdated Show resolved Hide resolved

validphys2/src/validphys/covmats.py Outdated Show resolved Hide resolved

validphys2/src/validphys/covmats.py Outdated Show resolved Hide resolved

andreab1997 added 6 commits March 10, 2022 14:12

removed old runcards and added new working one

99ffdd4

Fixed conflicts

f1ac85b

Fixing conflicts

6e64443

Removing comments

2601ce6

Fixing stuffs

9a8cf90

Changing n3fit_exec

88adcad

andreab1997 requested review from scarlehoff and Zaharid March 11, 2022 14:17

Merge branch 'master' into fix_thcovmat_fit

b7cf514

andreab1997 added 3 commits May 30, 2022 16:50

Fixing conflict

1d68e85

fixed conflict again

a922e26

Merge branch 'master' into fix_thcovmat_fit

62392de

andreab1997 added 2 commits May 30, 2022 17:36

Rerunning the tests

7d8f4e8

Merge branch 'fix_thcovmat_fit' of github.com:NNPDF/nnpdf into fix_th…

bbd3bc0

…covmat_fit

scarrazza requested a review from RoyStegeman June 27, 2022 11:29

RoyStegeman reviewed Jun 30, 2022

View reviewed changes

Some minor corrections

d84bb2f

NNPDF deleted a comment from andreab1997 Jul 1, 2022

andreab1997 added 3 commits July 1, 2022 12:46

Other minor changes

e3d41be

Final fixes

d4b4702

Reformatting funcs

4d768f0

scarlehoff reviewed Jul 5, 2022

View reviewed changes

validphys2/src/validphys/config.py Outdated Show resolved Hide resolved

removed file control

1c5205a

Implemented check of files

a10450e

andreab1997 merged commit 3098d0a into master Jul 6, 2022

andreab1997 deleted the fix_thcovmat_fit branch July 6, 2022 22:25

scarlehoff mentioned this pull request Sep 16, 2022

Re-enable use_t0 flag. #1599

Closed

scarlehoff mentioned this pull request Mar 14, 2023

Theory covmat prescription still determined by length of theoryids #1182

Closed

RoyStegeman reviewed Apr 1, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fit with theory covmat with n3fit #1528

Fit with theory covmat with n3fit #1528

andreab1997 commented Feb 22, 2022 •

edited by scarlehoff

Loading

scarlehoff left a comment •

edited

Loading

Zaharid commented Mar 10, 2022

Zaharid commented Mar 10, 2022

Zaharid commented Mar 10, 2022 •

edited

Loading

scarlehoff commented Mar 10, 2022

andreab1997 commented Mar 10, 2022

scarlehoff commented Mar 10, 2022 •

edited

Loading

andreab1997 commented Mar 11, 2022

scarlehoff commented Mar 11, 2022

alecandido commented Mar 11, 2022 •

edited

Loading

andreab1997 commented Mar 11, 2022

andreab1997 commented Mar 11, 2022

scarlehoff commented Mar 11, 2022

Zaharid commented Apr 7, 2022

andreab1997 commented Apr 29, 2022

andreab1997 commented May 30, 2022

scarrazza commented Jun 27, 2022

RoyStegeman commented Jun 27, 2022

RoyStegeman Jun 30, 2022

RoyStegeman Jun 30, 2022

scarrazza commented Jul 6, 2022

andreab1997 commented Jul 6, 2022

RoyStegeman commented Jul 6, 2022

andreab1997 commented Jul 6, 2022

RoyStegeman Apr 1, 2023

andreab1997 Apr 2, 2023

RoyStegeman Apr 2, 2023 •

edited

Loading

andreab1997 Apr 2, 2023

RoyStegeman Apr 2, 2023 •

edited

Loading

andreab1997 Apr 3, 2023

RoyStegeman Apr 3, 2023 •

edited

Loading

RoyStegeman Apr 3, 2023

andreab1997 Apr 3, 2023 •

edited

Loading

RoyStegeman Apr 3, 2023

Fit with theory covmat with n3fit #1528

Fit with theory covmat with n3fit #1528

Conversation

andreab1997 commented Feb 22, 2022 • edited by scarlehoff Loading

scarlehoff left a comment • edited Loading

Choose a reason for hiding this comment

Zaharid commented Mar 10, 2022

Zaharid commented Mar 10, 2022

Zaharid commented Mar 10, 2022 • edited Loading

scarlehoff commented Mar 10, 2022

andreab1997 commented Mar 10, 2022

scarlehoff commented Mar 10, 2022 • edited Loading

andreab1997 commented Mar 11, 2022

scarlehoff commented Mar 11, 2022

alecandido commented Mar 11, 2022 • edited Loading

andreab1997 commented Mar 11, 2022

andreab1997 commented Mar 11, 2022

scarlehoff commented Mar 11, 2022

Zaharid commented Apr 7, 2022

andreab1997 commented Apr 29, 2022

andreab1997 commented May 30, 2022

scarrazza commented Jun 27, 2022

RoyStegeman commented Jun 27, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

scarrazza commented Jul 6, 2022

andreab1997 commented Jul 6, 2022

RoyStegeman commented Jul 6, 2022

andreab1997 commented Jul 6, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RoyStegeman Apr 2, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RoyStegeman Apr 2, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RoyStegeman Apr 3, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andreab1997 Apr 3, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andreab1997 commented Feb 22, 2022 •

edited by scarlehoff

Loading

scarlehoff left a comment •

edited

Loading

Zaharid commented Mar 10, 2022 •

edited

Loading

scarlehoff commented Mar 10, 2022 •

edited

Loading

alecandido commented Mar 11, 2022 •

edited

Loading

RoyStegeman Apr 2, 2023 •

edited

Loading

RoyStegeman Apr 2, 2023 •

edited

Loading

RoyStegeman Apr 3, 2023 •

edited

Loading

andreab1997 Apr 3, 2023 •

edited

Loading