Conversation
|
when adding the new basis in vp2 I ve reformatted the file using black, not sure if that was a good idea. The only actual modification I did to |
| """ | ||
| if fitbasis == 'NN31IC': | ||
| if fitbasis == "NN31IC": | ||
| return np.identity(8) |
There was a problem hiding this comment.
If the output is to be 9 elements now it has to be 9 elements everywhere.
I'm guessing this will require changes anywhere the 8 is set by hand (which is potentially more places than just here)
|
While I'm usually in favour of passing all files through black, the matrix-like shape of the transformation at the end of the file would be helpful to keep... black creates a bit of a mess there (and just because of the size of the line). I'm happy with black but make sure it doesn't modify those lines (might be enough with edit: ups, clicked close instead of comment :_ |
|
@tgiani indeed please don't reformat lines that haven't otherwise being touched. It makes reviewing this a pain and messes up the history. Not to mention that the formatting gets worse in a number of places. |
|
Have you tried doing a first fit with this basis for Theory 200 ? |
|
@scarlehoff I m doing it now |
|
should be ready soon, if not today tomorrow |
|
Great! (I was going to offer doing it myself otherwise :P) Other than that the PR looks good to me. We can probably merge once we have both the fitbot and your fit. |
|
ok, maybe I could add also the corresponding flavour basis where we fit c and cbar? |
|
also, the report for validphys should be modified to add v15 I guess, should i do that in this PR? |
Since (for now) the 8-flavour basis is still the default I would leave the comparefits like it is (at least for this PR). This is what we do already with perturvative charm (the plot is in the 8 basis). We might want to add the plot in whatever the fit basis was whenever the fit is done with a non-default basis though, but I would separate it from this. |
|
ok, but here we will have to look at 9 pdfs at the end, so we do need to have an option for plotting V15 I would say? But yes, this can be done in another PR |
Personally I would do evolution / flavour / fitbasis (only when it is different) rather than adding V15 everywhere. |
|
Greetings from your nice fit 🤖 !
Check the report carefully, and please buy me a ☕ , or better, a GPU 😉! |
|
here s the report for the fit https://vp.nnpdf.science/UMM4lusgR-S8ktV-ie-5iA==/ |
|
I think the sum rules need to be actually updated https://vp.nnpdf.science/UMM4lusgR-S8ktV-ie-5iA==/#sum-rules (but the fit itself is great, the change in the sum rules is small and the chi2 is equivalent!) |
|
uhm shouldn t the sumrules be the same tho? Even if we are fitting v15 we should still have |
yes! there are no charm valence quarks in the proton (same as there are no strange) |
|
and here there are some additional plots for c, cbar, c+ and c- i ll fix the tests, need to update the sumrules test in vp2 |
|
Oh, wow. It looks great! We might want to redo it with Th 400 (but maybe we need to undo some of the optimisations done to the fktables!) Anyway, thanks! |
|
Do you understand why the agreement with pseudodata (E_val and E_tr) has improved significantly while agreement to the central data remained largely unchanged? |
|
The improvements are within the error though |
|
Oh actually nvm, it's still being compared to the fit from 2107. Compared to a more recent baseline fit, none of the chi2 values have really changed (as you would expect I guess): https://vp.nnpdf.science/y_AjuiH2RHO3QGI9Z97tBQ==/ |
|
how do I fix the vp2 regression test for the sumrules? I mean, I have to add the etries for c and cbar in the regression test files like |
|
The sumrules are computed so it should be fine just regenerating the If you want to add (also) a regression test with the ccbar asymmetry (I'd be in favour) you need to create a new low-precision fit for that with theory 162. |
|
ok thank you I ll do that. A part from this in this PR there s no other things to do right? How do we proceede to create theory 400? |
|
also I was thinking that it might be interesting to implement an additional sumrule for |
I would say so.
@cschwan @andreab1997 the optimizations for the non-independent flavours (when we collapsed V15 into V) is done only to the fktables right? The ekos and the grids contain all the information, so nothing needs to be recomputed, just reapply the ekos with a different set of optimization rules. Right? |
|
@scarlehoff I believe this is true; we discussed at some point to implement these optimizations at the level of the EKOs, but this was never done (probably better). |
Yes I believe this to be the case as well. |
scarlehoff
left a comment
scarlehoff
left a comment
There was a problem hiding this comment.
lgtm, don't know whether @Radonirinaunimi wants to have a last look
from my side once the bot is done I'd be happy with merging it
|
I've had a second look and everything LGTM. |
|
Greetings from your nice fit 🤖 !
Check the report carefully, and please buy me a ☕ , or better, a GPU 😉! |
|
@tgiani feel free to merge it if you are happy with it |
|
Is this going to change the fits by default? If so, can it be enabled conditionally? |
|
The default fit is going to be Also, now the sum rule for (edit, note that the only regression test that has been updated is the one for the sum rules because now there is an extra entry) |
|
Can we merge this? There's nothing missing right? |
|
ah yes sure, forgot to merge. Doing now |
8 participants
addressing #1627