Conversation
Hi @J-M-Moore , thanks for this. Could you please add some more description / update the description of the PR ? Also are there some checks/benchmark we can perform before merging this? |
J-M-Moore
changed the title
|
It seems fine to me, I don't see mistakes. The easy benchmark is to compare the two alpha_EW datasets, the one that contains the th_covmat in the commondata and the other by adding it in the fit |
|
@J-M-Moore or @comane Could you run a dumb fit of just cll in alphaEW, fixed PDF, in the two scenarios and verify that the results are the same? Than we can merge it. |
| spec_dict["invcovmat"] = invcovmat | ||
|
|
There was a problem hiding this comment.
Hi @J-M-Moore, here you are changing the invcovmat entry of spec_dict.
What about the covmat one, is that not needed?
I am asking since out_exp at line 378 is given the covmat not augmented with the theory one.
There was a problem hiding this comment.
Hmm yeah you're probably right that this is a mistake
| obsrot_vl = None | ||
|
|
||
| if use_th_covmat: | ||
| data_path = str(_get_nnpdf_profile()['data_path']) + 'theory_' + theoryid.id + '/simu_factors' |
There was a problem hiding this comment.
We probably need to have a check here to ensure that only the SIMUnet theory is used.
No other theory has simu_factors
|
What is the status of this @J-M-Moore, are we planning on merging it or is this not needed anymore? |
3 participants
This PR implements the theory covariance matrix supplied in the simu_fac files. Currently only does things at the level of the fit, we also need to add this to the chi2 computation and the plots.