Skip to content
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

Remove completely kinematics_override #2266

Merged
merged 12 commits into from
Feb 4, 2025
Merged

Conversation

scarlehoff
Copy link
Member

@scarlehoff scarlehoff commented Jan 28, 2025

This PR closes #2252, which has required datasets that either have only legacy version or for which it was just not done during the translation.

For HERA data in particular the situation was a bit dire since the plots reported Q2 but the numbers were actually Q. Now it should be Q2 everywhere, which might not be what everybody wants, but at least is correct.
The same is true for the NMC (with a horrible result, but the previous situation was just silly so I've modified the kinematics for that) and NUTEV.

(personally I'd move all the squared quantities to just... not the square?)

Edit: the fecking NMC legacy dataset with kinematics done with a dice had of course to be part of every single regression test 🤷 I hope we can move to the hepdata-binned version.

@scarlehoff scarlehoff added the run-fit-bot Starts fit bot from a PR. label Jan 29, 2025
Copy link

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

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

Copy link
Member

@Radonirinaunimi Radonirinaunimi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you very much for this (especially for fixing the remaining quirks of the old re-implemented sets)!!

So when would the k_ be completely stripped out of the codes? And do you perhaps happen to have a comparison of the kinematic plots?

validphys2/src/validphys/process_options.py Show resolved Hide resolved
validphys2/src/validphys/plotoptions/core.py Outdated Show resolved Hide resolved
@scarlehoff
Copy link
Member Author

yes, the k will (in a subsequent PR) all be removed. I cannot do it here because internally validphys uses the f**** kn everywhere so it requires some care.

Regarding the plots, I do have comparisons for the entire set of data. imho when they have changed they look better but I don't want to clutter the server too much, as a surrogate you can look at this comparisons between this comparefit: https://vp.nnpdf.science/a0kifkAWSlSTtrwUa7KqbQ==

and this older one: https://vp.nnpdf.science/eB5jnBi4Qn-OEc2KCq_i1g==

(if you want to look at the whole set of changes I can upload them to the server)

Copy link
Contributor

@giacomomagni giacomomagni left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks cleaner!!

validphys2/src/validphys/kinematics.py Outdated Show resolved Hide resolved
@Radonirinaunimi
Copy link
Member

yes, the k will (in a subsequent PR) all be removed. I cannot do it here because internally validphys uses the f**** kn everywhere so it requires some care.

Okok, good!

Regarding the plots, I do have comparisons for the entire set of data. imho when they have changed they look better but I don't want to clutter the server too much, as a surrogate you can look at this comparisons between this comparefit: https://vp.nnpdf.science/a0kifkAWSlSTtrwUa7KqbQ==

and this older one: https://vp.nnpdf.science/eB5jnBi4Qn-OEc2KCq_i1g==

As far as I can tell, everything looks good.

(if you want to look at the whole set of changes I can upload them to the server)

Maybe even just for the record, this is good to have on the server in any case.

@scarlehoff
Copy link
Member Author

scarlehoff commented Feb 1, 2025

Maybe even just for the record, this is good to have on the server in any case.

I put them in /home/nnpdf2/nnpdf/PR_tests/PR_2266_removal_of_kinematics_override (this nnpdf2 is a second disk we got since the first one was full, I've started moving things around but still lots to do). The folders are separated by <experiment>_master which is run with master and <experiment> which is run with this branch. To first approximation I ran them commit by commit (so the LHCB folder refers to the LHCB commits and so on).

@scarlehoff scarlehoff removed the run-fit-bot Starts fit bot from a PR. label Feb 3, 2025
@scarlehoff scarlehoff force-pushed the fix_kinematics_override branch from 7ff2738 to 698521b Compare February 4, 2025 10:20
@scarlehoff scarlehoff merged commit 91226b2 into master Feb 4, 2025
9 checks passed
@scarlehoff scarlehoff deleted the fix_kinematics_override branch February 4, 2025 11:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove kinematics_override.
3 participants