NNPDF
Make the diagonal fit the default
fwiw, I think (regarding 338cf0d) that perhaps is not the best idea to mix changes here.
Given the urgency (I just read the minutes and apparently people would like to start already with mhou) we should focus on simply extracting the diagonalization so that it is not performed once per replica.
The change of the inverse covmat by the cholesky can be done transparently on its own and is a completely separated issue.
(also, please rebase since this branch might by now be far away from master)
I have removed the commits related to the Cholesky decomposition from this PR. I have also rebased on top of main
Thanks. I think many of the tests will need to be manually changed (some of them to assume a non-diagonal covmat despite the default, some of them to work with diagonal covmat).
Did you try whether it worked as expected? (if you run a diagonal fit in parallel the memory doesn't explode / the inverse is not computed every time?)
I tried running a MHOU fit (in order to try and get the baseline already with this) but it crashed at the vp-setupfit stage. I think this might be related with some of the errors in the tests.
This should hopefully be addressed now, thanks for letting me know! Let's see what the tests will complain about.
I'm afraid in this case you need to do some manual work for the regressions.
On the one hand, you need to decide which ones you want to do now diagonal and for which you want to keep the non-diagonal (at least one we want to keep).
On the other, you have the regressions within the tests of validphys and n3fit, which might be failing for other motives and not just because the fitting is stopping at a different point. Even if that's the only failure mode, those are not in the redo-regressions part, so you need to run those tests manually in your computer (fwiw, those are intended to be run also locally to be able to quickly check yourself, while the regressions that are automatically autogenerated in the CI are intended to be run only in the CI).
test_pseudodata now passes, but test_regressions.py not yet. I have generated the regression data using pytest extra_tests/regression_checks.py --regenerate True, and this updates all files in extra_tests/regression_fits, but not those in validphys2/src/validphys/tests/regressions. How do I update those?
The reason these need to be updated is because many csv files come in (group, dataset, id), but in the diagonal basis we only have (group, id). The dataset has lost meaning because of the change of basis.
The ones in the extra tests folder can be regenerated with the bot as they are very strict.
For the ones in validphys you can remove the files, then run the tests, and then the files will be regenerated.
Greetings from your nice fit :robot: ! I have good news for you, I just finished my tasks:
- Fit Name: NNBOT-33ed29fc1-2025-11-10
- Fit Report wrt master: https://vp.nnpdf.science/P8ANbtXuRpyISr69emCq4A==
- Fit Report wrt latest stable reference: https://vp.nnpdf.science/sPvObLo4ReWo2-PJaLsSeA==
- Fit Data: https://data.nnpdf.science/fits/NNBOT-33ed29fc1-2025-11-10.tar.gz
Check the report carefully, and please buy me a :coffee: , or better, a GPU :wink:!
![github-actions[bot] avatar](https://avatars.githubusercontent.com/in/15368?v=4 "Published by a pub.dev verified publisher"){kind=small}
Hi @scarlehoff, @jacoterh, here is the report of the default diagonal fit test on the latest settings (251105-jcm-nnpdf41-mhou-001 runcard): https://vp.nnpdf.science/kSXuJUNETmCmPFoZM8oY2g==
Hi @scarlehoff , there's something weird going on in the pseudodata test on master. Somehow, it doesn't take into account correlations across datasets and draws replicas only dataset by dataset, while this PR draws the pseudo data from the full covariance matrix. Therefore, the test test_read_matches_recreate which attempts to recreate the pseudodata that got saved during fit pseudodata_test_fit_n3fit_250616 behaves differently on master compared to this branch.
Specifically, https://github.com/NNPDF/nnpdf/blob/38e38bfb67635562e1878b18521b8dc8bf3b89c1/validphys2/src/validphys/pseudodata.py#L211
gets called for each dataset when running pytest test_pseudodata.py on master , while it only gets called once on this branch (assuming a single replica). I found this after looking more closely into why I had to regenerate pseudodata for the tests to pass again.
This makes sense, right? It should be taking it experiment by experiment and then eventually a dataframe is created separated by dataset (which I guess it is what the test is saving)
https://github.com/NNPDF/nnpdf/blob/38e38bfb67635562e1878b18521b8dc8bf3b89c1/validphys2/src/validphys/pseudodata.py#L218C1-L219C1
Since pseudodata_test_fit_n3fit_250616 is old, you need to assume this was done with the old default, so you should test diagonal: off
Actually, I think it is great that it failed, you want test_read_matches_recreate both for diagonal and not diagonal! So you can keep the non-diagonal one and add a diagonal one, to make sure that both are correct.
That said, you might want to also add a version check in fit.as_input() (like here so that when the version is older than 4.1.2, the diagonal matrix is set to false (we will tag as soon as this is merged)
Greetings from your nice fit :robot: ! I have good news for you, I just finished my tasks:
- Fit Name: NNBOT-04e1dc458-2025-11-18
- Fit Report wrt master: https://vp.nnpdf.science/Mfbjc75oSOi63AOSydIWNg==
- Fit Report wrt latest stable reference: https://vp.nnpdf.science/dQV9yhmWQWONjSDTDwTL2A==
- Fit Data: https://data.nnpdf.science/fits/NNBOT-04e1dc458-2025-11-18.tar.gz
Check the report carefully, and please buy me a :coffee: , or better, a GPU :wink:!
I've updated the fitbot, let's see how the new one looks.
I'm effectively off until Monday in two weeks, but I'll try merge and tag before. If you could deal with the reporting problem that would be helpful. Thanks.
I've updated the fitbot, let's see how the new one looks.
I'm effectively off until Monday in two weeks, but I'll try merge and tag before. If you could deal with the reporting problem that would be very helpful. Thanks.
edit: actually, the problem with the chi2 is not only the reporting since that is used for the chi2 check of postfit. Most fits have most replicas to be good so I guess it was hard to spot
Thanks, taking a look, the experimental is rotated here https://github.com/NNPDF/nnpdf/blob/9e681094f8669155a692a22676364de9fd6db50a/validphys2/src/validphys/n3fit_data.py#L478 so not sure what is causing this...
Indeed, by looking at that you rotated the experimental data... of the replicas. But not the central experimental replica against which each replica is tested at the end 😅 you have to do the same for expdata_true
Greetings from your nice fit :robot: ! I have good news for you, I just finished my tasks:
- Fit Name: NNBOT-f04bf06f2-2025-11-21
- Fit Report wrt master: https://vp.nnpdf.science/v9t562c4SWSlex5twRY58g==
- Fit Report wrt latest stable reference: https://vp.nnpdf.science/mbmDMXECQqyIOiBZIWrp3g==
- Fit Data: https://data.nnpdf.science/fits/NNBOT-f04bf06f2-2025-11-21.tar.gz
Check the report carefully, and please buy me a :coffee: , or better, a GPU :wink:!
Greetings from your nice fit :robot: ! I have good news for you, I just finished my tasks:
- Fit Name: NNBOT-6a415003c-2025-11-21
- Fit Report wrt master: https://vp.nnpdf.science/nUtHYNTjTHSfw9ubBMtbFw==
- Fit Report wrt latest stable reference: https://vp.nnpdf.science/T6unMvcUQfuZdCvEI3Sjrg==
- Fit Data: https://data.nnpdf.science/fits/NNBOT-6a415003c-2025-11-21.tar.gz
Check the report carefully, and please buy me a :coffee: , or better, a GPU :wink:!
Greetings from your nice fit :robot: ! I have good news for you, I just finished my tasks:
- Fit Name: NNBOT-7c6fc6f40-2025-11-21
- Fit Report wrt master: https://vp.nnpdf.science/WzVsGMfjRyOvtZ-L6FLPog==
- Fit Report wrt latest stable reference: https://vp.nnpdf.science/K477bPjCR7Gu9rpNxFq1PQ==
- Fit Data: https://data.nnpdf.science/fits/NNBOT-7c6fc6f40-2025-11-21.tar.gz
Check the report carefully, and please buy me a :coffee: , or better, a GPU :wink:!
Greetings from your nice fit :robot: ! I have good news for you, I just finished my tasks:
- Fit Name: NNBOT-99108504e-2025-11-22
- Fit Report wrt master: https://vp.nnpdf.science/usCnvmGQSNyUckUHjzYxzg==
- Fit Report wrt latest stable reference: https://vp.nnpdf.science/sLJmEGUsSL2rpuyd4Cb_Cg==
- Fit Data: https://data.nnpdf.science/fits/NNBOT-99108504e-2025-11-22.tar.gz
Check the report carefully, and please buy me a :coffee: , or better, a GPU :wink:!