Refactor xintegrator #1779
Refactor xintegrator #1779
Conversation
|
This looks good to me (it removes some extra-operations and have tests)! But I don't really understand how this would make easier the inclusion of nPDFs?
In general, I am for atomic changes, but in order to not lose track of the bigger picture/aim shouldn't these be included in the respective PRs -- and especially if they are really needed? |
|
Please don't merge this yet. |
|
@Radonirinaunimi The reason it helps is that now as long as the x axis stays the same it works for pdfs of any shape, so you can add an A axis. |
|
@scarlehoff Is it ok to merge now? I did some tests running for a few epochs. The numerical noise is different so you get very small differences that grow every epoch. I don't think anything short of a full multi replica fit so you can see the distribution will give more checks than the unit test. |
There was a problem hiding this comment.
The numerical noise is different so you get very small differences that grow every epoch
That's strange, from the changes to the code I'd expect the changes to be identical.
Are you sure that the changes are due to this? Do you have an idea why could it be?
(in any case, the report by the bot should be done in a few hours, we can merge then)
|
Greetings from your nice fit 🤖 !
Check the report carefully, and please buy me a ☕ , or better, a GPU 😉! |
|
Seems exactly the same to me https://vp.nnpdf.science/3G193e67SqS3DlDHqwUEeg==/#Scales0_Normalize_Basespecs0_PDFscalespecs0_Distspecs0_plot_pdfdistances maybe you saw some differences because a different version of some library ? |
Adds a regression test for
xIntegratorand refactors it to use a tensor product on the x-axis rather than replicating the grid tensor.I expected it to be faster but the difference is negligible (few % faster).
The main point though is in preparation for other changes. I think @Radonirinaunimi 's inclusion of the nuclear pdfs will benefit from this, as well as a possible optimization for multi-replica fits I'm working on.
I'm trying to put changes that will be beneficial regardless in their own small PRs, I think that'll be easier to review. (Not a lot, I think just 2 more)