-
Notifications
You must be signed in to change notification settings - Fork 2
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
Positivity #146
Positivity #146
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #146 +/- ##
===========================================
+ Coverage 68.74% 68.82% +0.08%
===========================================
Files 82 82
Lines 3996 4007 +11
===========================================
+ Hits 2747 2758 +11
Misses 1249 1249
Flags with carried forward coverage won't be shown. Click here to find out more.
|
This part should be implemented as a suitable runcard, not in |
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 had a look and everything seems good to me. However I am not that much familiar with yadism so I cannot really say that I have understood all the changes you have done.
actually, yes and no: of course everything will eventually end up in runcardsrunner (as said): NNPDF/pinecards#143 - but it will be just a plain copy (as for the other DIS experiments); first I'd like to take advantage of the data package here which allows me an easy benchmark |
@enocera thanks for your info! I hope I implemented the python equivalent here - however when running a simple, LO benchmark against apfel it is complaining " Invalid value of x = 4.9999999999999998E-007" (full output below) ... do you know if they (=apfelcomb) hacked apfel in some way to run it (i.e. extend it's default grids)? in any case I try to benchmark against the old FK table ... WARNING: ZM-VFNS is a VFN scheme ... setting VFNS PDF evolution
Allowed evolution range [ 1.0000 : 10000.0000 ] GeV Initialization of the evolution completed in 0.140 s Report of the electroweak parameters: Mass of the Z = 91.188 GeV Report of the DIS parameters: Computation in the ZM-VFNS mass scheme Initialization of the DIS module completed in 0.018 s In F2light.f: |
@felixhekhorn This is strange. If I run apfelcomb it runs just fine. And as far as I know, apfel has not been hacked for the purpose of positivity observables. Have you tried to sample the x grid, instead from x=5e-7, from 5.000001e-7? |
@enocera yes I tried, and this does not work either - in fact I believe the minimum x-grid value in apfel is 1e-5 as defined here: https://github.com/scarrazza/apfel/blob/4426f689a95fbde94012083dd55a9af0edcc5928/src/Evolution/initParameters.f#L89 and indeed this works PS: since we're speaking of it and you might be interested in the future: our default x_min is 2e-7 as defined by the PineAPPL paper https://github.com/N3PDF/yadism/blob/ecc16f965504435423e44868790cf5d154e0d7b2/extras/data/machinery/generate/utils.py#L12 |
alright, there are strange things happening ... someone has an idea? @alecandido @cschwan @scarlehoff @enocera the summary is:
check against apfelif I fix the x_min problem, by either putting the cut temporarily to 5e-5 or by forcing APFEL to use my grid (which has support in that region) I'm getting something reasonable:
the old FK tablewhen I look into the NNPDF FK table I can see problems:
comparing new against old inside vp
comparing new against old with pineappl
|
Then I might doubt this statement: the version of APFEL used to produce old FkTables is not the same as yours. In the meanwhile a few things have been changed... (in principle not so relevant)
On the other side, APFEL is Evolution + DIS. You agree with DIS, so the problem might still be inside evolution (at the level of FkTables). |
@felixhekhorn (cc @scarlehoff @alecandido). Let me make some remarks.
|
Since the comparison seems alright - I'm ready to merge; @alecandido whenever you have time, please review and we merge
PS: in the end the non-matching was just due to not using the correct version inside pinefarm 🙈 thanks @scarlehoff for making me realize |
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.
First batch, I still have to check most of the part in the yadism
package itself.
Mostly trivial stuffs.
Overall, there are only minor corrections and a few clarifications to be provided. |
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.
Proposed a tiny improvement. But don't mind about this right here: even if you'd do it immediately, first merge, then fix in develop
.
🎉 |
F2up(x=?,Q2=?)