Refactor rotations

[APJansen]

This is a small refactor of the rotation layer, now requiring a rotation_axis argument that will be used to take a tensor product with that axis from the input and the 0th axis of the rotation matrix. (the resulting axis is by default put at the end, so this is transposed back afterward).
It's more intuitive than the previous axes which specified the amount of axes to contract (starting from the end of the input and the start of the rotation matrix).
More importantly, it now requires no change when axes are added to the pdf.

I've added a regression test to make sure results are still the same.

[scarlehoff]

Did you test this with different runcards, sum rules on off, different basis (not only flavour and evol but also perturbative charm and ccbar asymmetry), etc?. The same comment applies to the other PR, depending on what you are modifying different tests will be needed.
For many of these tests is enough to check that results are equal for one single replica but these checks need to be done.

There are people working with the code using some of these options for whom a change in the results would void many hours/days/weeks of work so I'm happy about the small PRs of course but all these options should be tested otherwise the fact that they are small doesn't really help

[APJansen]

Well that's why I'm making small PRs with regression tests. I think all those cases are covered by the single regression test I added. Note it only covers FkRotation, not FlavourToEvolution, but the actual change is in the base Rotation layer. I could add tests for FlavourToEvolution if you prefer.

But indeed if despite these tests you think it still requires the whole suite of runcard tests then there's no point in small PRs, on the contrary our resources wouldn't allow it I believe.

[scarlehoff]

I don't think these changes need full fits, but just running a single replica and ensuring that results are unchanged in all cases.

then there's no point in small PR

I meant the opposite. Having small PRs allows for testing the incremental changes separately and thus it should be done.

edit: also, don't get me wrong, I like the changes and they seem positive on a first look (sadly I don't have time to review them and won't have in the near future) but in the current state of affairs (with several papers about to be written) that results don't change in suprising ways is extremely important.

[APJansen]

Ok I understand, good to know that you are busy the coming time.

So which runcards exactly? All in examples/, and the 3 main ones I tested for the model_refactor PR? That is, NNPDF40_nnlo_as_01180_1000, feature_scaling and flavor_basis?

I have 4 small PRs (refactor_xintegrator, refactor_msr, refactor_preprocessing and refactor_rotations).
They all come with regression tests and should give the exact same results, and they're all independent. So if I go through all those runcards on a temporary branch that merges these 4, and everything remains the same, would that be sufficient?

These 4 are all for the moment by the way, the next thing I want to try will depend on Gijs's #1661 and will require lots more testing. Or maybe one more before I do the tests...

[Radonirinaunimi]

Hi @APJansen (@scarlehoff here)

So which runcards exactly?

Do you have access to the NNPDF wiki? I think this would be much faster if you had (that way we can point directly to relevant runcards with associated reports so you can have a look) (I'll tell @Radonirinaunimi to help you to get access to it if you don't).

But more in general, since you are modifying the very internals of the code I think it would be good if you familiarize yourself with the different options in the runcard in order to have an idea of what could be affected for each change (for instance, for the change in the integrator I would think fiatlux and parallel replicas might be affected but not changes of basis). That way you can iterate much faster (instead of us having to go through the process of looking at the changes, coming up with possible changes, etc). That would also help creating regression tests that cover all possibilities of course.

[APJansen]

@scarlehoff If you mean to https://vp.nnpdf.science/ then yes I do, don't know of any other source other than the documentation.

[APJansen]

Here the result is really identical, without numerical noise. I checked the validation chi2 for the first 100 eopchs, comparing this with master, and the results are identical. For the basic runcard, and for the flavor basis one.

[scarlehoff]

Thanks for the tests. As far as I can see this should be safe to merge save the comment about the naming.

For the other future changes, you can have a look at this page:

https://www.wiki.ed.ac.uk/display/nnpdfwiki/NNPDF4.0+final+fits

under "methodological variations", FM10 to FM17 are the runcards that can potentially be affected by the changes to n3fit.
Also FT03 (perturbative charm). Not all will be affected by all changes, just have a look and ask @Radonirinaunimi if you are not sure what some of them mean.
Maybe it would be good to add them all to some folder with O(10) epochs and just making sure that the code runs since most of the changes that would break them will probably make the code crash.

If I have time I'll add that last part as a test, that way we'll avoid a lot of the "test this, test that". Sadly it won't ready for this first batch of PRs.

Anyway, if you haven't got access to the wiki yet (it's summer so it's not a given that the people in charge is available) I can send the runcards to you via mail (alongside their descriptions).

The QED fits are not included there but let's hope that any possible breakage is cover by the regression tests. Also because checking the QED fit for every change in a proper way is probably not feasible.

RE the fits with the "standard settings" you can just tag the fitbot for that. That way we also have a link with a report for future references.

[APJansen]

No I don't have access to the wiki yet unfortunately, I'll have a look once I get it. And that'd be great, to have those included as a test.

[github-actions]

Greetings from your nice fit 🤖 !
I have good news for you, I just finished my tasks:

• Fit Name: NNBOT-b6e6ad034-2023-07-28
• Fit Report: https://vp.nnpdf.science/PUYFV5aySQK8homLZ905xw==
• Fit Data: https://data.nnpdf.science/fits/NNBOT-b6e6ad034-2023-07-28.tar.gz

Check the report carefully, and please buy me a ☕ , or better, a GPU 😉!