-
Notifications
You must be signed in to change notification settings - Fork 7
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Polarised proton fits #1979
Polarised proton fits #1979
Conversation
@Radonirinaunimi it would be good to rebase again since the changes to the convolution recently implemented will have an effect here (but they also simplify a lot what needs to be done, in that everything will be much more clear) |
Yep, I am doing so at the moment. Hopefully this will not be painful 🙃 |
It was slightly tricky but I believe it works. I will clean up a bit while adding some docs and tests. |
Regenerate test pol
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is good to go. Thank you for the hard work! I like how they are changes that are relatively major for how the fit works but are small nudges to the code ^__^
The only thing left for me to be happy would be having a fit + report with this latest version of the code (after comments are applied or ignored in case-by-case basis xd) before merging
I already read your mind earlier and the evolutions are running atm xd Will generate some reports tomorrow while addressing your suggestions. |
The reports are now failing because of some recent changes in the computation of stats that appear following lines: nnpdf/validphys2/src/validphys/pdfplots.py Lines 617 to 619 in aeb2a96
I am fixing this now. |
What changes? |
That should only happen if you are getting a list and not an array, and that should definitely not happen because the nnpdf/validphys2/src/validphys/core.py Line 828 in aeb2a96
So there might be something wrong somewhere else? |
With possibly some improvements in the Here are some reports:
PS: These will be the fits we'll send to Stagnitto et al. |
Greetings from your nice fit 🤖 !
Check the report carefully, and please buy me a ☕ , or better, a GPU 😉! |
Improve sumrules module a bit by splitting operations
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The fitbot corresponding to the last commit #2057 (comment)
This PR implements the necessary changes to perform polarised fits. It deprecates the intial implementation in #1893.
Remaining Todos:
test_fit
, requires Polarised DIS and DY Commondata #1816 to be merged and then rebased here.)fix positivity dataset plotsno longer needed as superseded by BC plots