Skip to content

Conversation

@PiaLJP
Copy link
Contributor

@PiaLJP PiaLJP commented Oct 20, 2025

The number of priors needs to be included in the expected chisquare, see eq. (D.9) in section "D.2 Priors and the Bayesian approach" in arXiv:2209.14188v2 ("On fits to correlated and auto-correlated data").
Note that the expected chisquare is now correct in the case of priors that are not correlated with the data fitted.
For correlated priors further alterations would be needed. (The covariance (from eq. (D.9)) is no longer be diagonal w.r.t the priors so you cannot just add the number of priors.)

@PiaLJP PiaLJP requested a review from fjosw as a code owner October 20, 2025 15:57
@fjosw fjosw requested a review from s-kuberski October 20, 2025 17:51
@s-kuberski
Copy link
Collaborator

Hi Pia,

thanks a lot for adding this, it has been on my list for quite some time now.

I agree that, the code will still not be correct in the case where the prior is correlated with the data. This should be addressed at some point. I had started to implement the proper solution, but then I was hesitant because, in principle, also the notion of a correlated fit would then have to be changed to a covariance matrix that also covers the contribution of the priors, so both cases should be handled together.

For now, it is good to fix the obvious bug for the standard case of an external prior.

@s-kuberski s-kuberski closed this Oct 21, 2025
@s-kuberski s-kuberski reopened this Oct 21, 2025
@s-kuberski
Copy link
Collaborator

Sorry, I closed this by mistake. Before merging it, would it make sense to write a small test that ensures thatchisquare_by_dof for an uncorrelated fit to uncorrelated data, including priors, matches chisquare_by_expected_chisquare?

@PiaLJP
Copy link
Contributor Author

PiaLJP commented Oct 21, 2025

Thanks :) , I just added such a test, also included a combined fit example where I also hand over a cov without correlations, maybe it's a little redundant and the first fit to a constant is sufficient and I should delete the rest?

@s-kuberski
Copy link
Collaborator

Thanks for the addition! I think there is no need to remove anything. I had briefly thought about also doing a fit with two parameters and two priors, but I think it is sufficient as it is right now.

@fjosw fjosw merged commit e0076cc into fjosw:develop Oct 22, 2025
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants