NNPDF / pinecards

Runcards needed to generate PineAPPL grids for NNPDF processes
3 stars 1 forks source link

Add runcards (and runner) for integrability observables #150

Closed scarlehoff closed 2 years ago

scarlehoff commented 2 years ago

I've only added the two we use in the evolution basis fit but it also works with the ones used in the flavour-basis fit.

I've already checked that the numbers are equal to those of the integrability fktables.

codecov-commenter commented 2 years ago

Codecov Report

Merging #150 (4c7238b) into master (54d4a55) will increase coverage by 0.66%. The diff coverage is 41.66%.

@@            Coverage Diff             @@
##           master     #150      +/-   ##
==========================================
+ Coverage   29.53%   30.19%   +0.66%     
==========================================
  Files          23       24       +1     
  Lines        1141     1212      +71     
==========================================
+ Hits          337      366      +29     
- Misses        804      846      +42     
Flag Coverage Δ
unittests 30.19% <41.66%> (+0.66%) :arrow_up:

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
pinefarm/external/integrability.py 41.17% <41.17%> (ø)
pinefarm/info.py 64.00% <50.00%> (-4.19%) :arrow_down:

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

scarlehoff commented 2 years ago

please remember to install pre-commit (consider that sooner or later also here we will opt into pydocstyle)

poetry install pre-commit ?

Moreover given that, changing Q0 is not completely out of scope

Actually, I guess it makes more sense if I take Q0 from the theory instead?

felixhekhorn commented 2 years ago

poetry install pre-commit ?

actually pre-commit and poetry conflict with each other ;-) (since each of them wants to manage a python env ... ) - so pre-commit should be (in some sense) installed globally - see also https://pre-commit.com/ ; actually in such a ways that git can access it

Moreover given that, changing Q0 is not completely out of scope

Actually, I guess it makes more sense if I take Q0 from the theory instead?

if you want to enforce the two numbers are always the same, yes

scarlehoff commented 2 years ago

This can be merged.

if you want to enforce the two numbers are always the same, yes

Yes I do. I've done that now.

scarlehoff commented 2 years ago

That's weird. I did run pre-commit before the last commit.

felixhekhorn commented 2 years ago

That's weird. I did run pre-commit before the last commit.

then this should not have happend ... mmm ... I was about to say "pre-commit also prevents stuff like this, which can be caused by different black version e.g. because it gets pinned"

felixhekhorn commented 2 years ago

That's weird. I did run pre-commit before the last commit.

@scarlehoff stupid question: you did activate pre-commit in the repo? What happens if you run pre-commit run --all-files?

scarlehoff commented 2 years ago

Nothing.

But I think it is because your previous pre-commit caused a conflict (I had not pulled) so I rolled back before pushing. Probably undid also the pre-commit. My bad.

scarlehoff commented 2 years ago

I'll merge this then? Or do you want me to change the x to 1.0 now that the pineappl bugfix is done?

felixhekhorn commented 2 years ago

either way ... @cschwan do we want to fix the affected datasets after closing https://github.com/N3PDF/pineappl/pull/167? (as said also https://github.com/NNPDF/runcards/pull/132 are affected)

in any case people are starting to run fits (e.g. @giacomomagni ) and they need all datasets

scarlehoff commented 2 years ago

Then I'll merge and i'll fix it at a later stage

cschwan commented 2 years ago

@felixhekhorn if something is wrong we should fix it!

felixhekhorn commented 2 years ago

@felixhekhorn if something is wrong we should fix it!

there is not necessarily something wrong on putting the trivial x2 point something different then 1.0 (it is not used after all), however I'd suggest to standardize and put them to 1.0, so that's what we would need to "fix" - I think integrability are the only pinecards affected (all others are at 1.0 (and that's why they fail ...))