First pass, add a PDF class#5
Conversation
|
Thank you @scarlehoff ! That sounds very good - I will have a go at running this to check it works for me and yes no problem just let me know about config files etc |
|
At the moment you need that specific branch (that needs newer versions of python). I'll make it work for 3.9 too |
|
thanks - the only reason 3.9 is used by default is I think to get lhapdf python to work. I will hold off for now then |
|
lhapdf, at least from conda-forge, should (since sometime last year) work for all python 3.9-3.13 In any case, I've modified the branch in nnpdf so it should now work with older versions too. |
|
Thanks - I have had a quick go and 'reportengine.compat' (yaml is imported from there in the main file) seems to be missing from the reportengine installation as per your README? I might be doing something wrong... If you want me to hold off running for now while you still make change np just let me know |
|
Fixed. There was a recent update to reportengine that I apparently forgot about. |
|
Just to say that is now running for me - the outputs are definitely not right and the runtime is actually pretty similar to before but that may well be because things are still work in progress. Let me know if I can help |
Perfect! For now the calls are correct, the content not so much because I'm still going through the different parameters to understand when/what things should change. I'm able to reproduce the results of the main branch by setting some stuff by hand* but my goal is to be able to understand the code enough to have it in automatic. *the first call must ignore the parin1 variable and instead use the pdf of PDFlabel_lhin as input, but not sure what the parin1 is at that point |
|
Btw, one thing that is not clear to me. The config file that you left creates 3 PDFs. Is the last (test_3 or something in that case) what would be the final PDF in the optimization? |
Indeed that is not very clear - if lhin is set to True in the config file then the LHAPDF grid with PDFlabel_lhin is used - so this is just an option to output a chi^2 for an arbitrary user-given grid. If you set it to false then `parin1' (which gives the PDF parameters) is used, as read in from the input file in the config (if lhin is True this is not used) |
The test_XX (where 'test' is whatever label is set in the config file) grids are the temporary ones that are created from the input file (if lhin is set to true this still happens, i.e. the lhapdf output is regridded, though that is not really necessary). As the default is not running a fit, only 3 are created - there are 3 runs to give the chi^2 in the t_0, experimental definitions and with/without positivity penalties. These should actually be deleted by the end of the code. If you ran a fit there would be many more (though again deleted as you go) due to the minimisation. The actual fit output is given by the code in |
|
ok, great. That helps a lot! And I have a related question, why do you create two points in Q when you create the grid? Is that just because LHAPDF complains otherwise or is there a deeper reason? Could you send me a runcard for a fit (perhaps the monte carlo version since that's what I'm more used to so it will be easier for me to follow, but both are fine)? To make sure I'm able to reproduce both the computation of the chi2 and the subsequent fit. |
Indeed there is nothing deep there - the LHAPDF routine complains if you don't have at least 2 Q points. I did check that the second higher one is completely redundant though, i.e. by setting the PDF values to zero there etc. It is not used if you are exactly on the first Q_0 point (which you are). I'm sure there is a work around within LHAPDF but that seemed like more trouble than it was worth...
Sure. In terms of a Monte Carlo version, I'm not completely sure of the distinction you mean here? The code performs a fit to a given set of data - or pseudodata. A MC fit would involve running multiple fits to multiple sets of pseudodata (as you know of course!). At the moment that is the kind of thing (to the extent that I have done MC fits, which is not too much, just for a few checks in the paper closure tests) that would be done via an external script. So I guess we really just need an input card with a chi^2 and an fit output to the unfluctuated real data as the simplest thing? To speed things up probably with just a couple of PDF parameters free as a first step? |
|
Ah, ok. I thought it might do the replicas automagically. Yes, something simple is enough, so that we can check the result is exactly the same up to numerics. |
|
No problem - I have created the branch |
|
I have another question, in Could the array It would be great if we could remove the side effects of at least the PDF functions (and ideally have them with the same signature so that we can abstract things out). (and ideally ideally have |
|
Hi yes there is no particularly deep reason that things are done like that - the minimisation with
Sure, there is definitely room for improvement! |
|
I see. *(I decided to start with that goal because it is clear that nothing should change so it is easy to check the final products) |
|
Ok thanks - in fact I was not completely clear in my last message I think. Just to clarify the argument which is actually not needed at all, and is just a leftover from old code. |
… still needed to finish the fit ; added the fit example runcards by Lucian
There was a problem hiding this comment.
Thank you! One immediate quick comment to save you time - you can assume that newmin is True from now on - if it is False this gives the older derivative (a brute force difference rather than using the form of the FK convolution) and there is no need to keep this. Similarly anything with diff_2 or diff_4 (which are options that only make sense for newmin being False) can just be deleted and certainly no need to update it.
|
Hi @LucianHL, I have a question. In |
|
Thanks!
Does that mean that the results are very different from RE: speed and derivatives I think part of the difference might be that some of the stuff we are using under the hood is trying to use multiple cores so it is penalized running in a single core. But there's still a lot of stuff that can be improved, in particular the derivative could be compiled with something like numba and the part that's calling directly the NNPDF code is using the routines that we use for plotting and that are very much not optimal for being called continously. |
Just a bit, i.e. the sort of thing you would expect given the codes are not identical. For e.g. the longmin case the old code gives a final
Ok understood. I am not familiar with numba but will have a look. As I say I think procedurally it makes sense long term to have the derivatives done at the level of the PDFs purely but this clearly depends on exactly how quick you can make the calls to the theory calculation itself. If that is no longer a bottleneck then for sure there is no need to write things in that way. |
That's even too close. I would've expected a bigger change. |
|
Indeed! Yes as I say I am pretty sure for other examples the differences may end up being larger than that! |
|
Interestingly, running the code in my computer I get an even better chi2i = 5710.46724429442 |
|
Hmm that is a bit surprising! Have you tried running the resulting fixed parameters (from |
|
I m going to try to run it as well on another machine, so we ll have a third datapoint |
|
Thanks - in the meantime @scarlehoff if you can pass on the final output parameter file I will have a quick look with that |
|
p.s. is this with the short or long min. Or both?? |
|
With long, short gives me something very close to yours: (I'm reading the output files from output/buffer) |
|
Thanks, yes that should give you the right answer. As I say if you can pass on the file from |
|
with short I get smt similar to Juan |
|
with long it is still running, but if I look at the temporary buffer I have so it seems like I will also end up with a lower chi2 than the one reported in |
|
Thanks ok - I have set a fit going starting directly with the latest version of this branch just to be 100% sure my previous quoted |
|
I get These should be the parameters |
|
Hi @LucianHL, after running the code using the runcards you provided we get the central value of the pdf, saved for example in |
|
Hi both something is not quite working with the newest code- hence why you are apparently seeing lower minima. I also reproduce that with the latest git version. I am pretty sure related to the experimental/$t_0$ covariance matrix question. I am looking into it now In particular, that output parameter set you have provided does not agree at all with the chi^2 values you are quoting @tgiani . Anyway, more to follow soon - sorry about that |
|
That should now be fixed. What was happening here was that the addition I had mentioned in a comment above was not included in this branch and so the t0 covariance matrix from the first input set was being used throughout the minimisation. So the lower chi^2 value was due to the fact that this t0 covariance matrix was being used, i.e. far from matching the final PDF parameters. You will see this if you pass the output from your fits back through the code (at which point the final PDF parameters will be used for the t0 covariance matrix) - i.e. the chi^2 will not match. This was fixed in the code I had been using and I had wrongly assumed we were matching. Anyway I will set a final longmin check fit going now just to see if it is starting to match what it should (though I have checked that with simpler fits), so if you like please wait a little for me to confirm things are looking good. I have also added a bit more info about running scans to the README |
|
Hi both that is working now for me, and the longmin run is giving a |
|
Hello, thank you, now I m also getting |
|
Hi @LucianHL , in order to run a scan to compute the PDF error using for example |
|
Excellent! Thanks, and sorry for the noise there.
The input file will not be used as |
|
I think at this stage we can move to the next step which is isolating the parts that are used from nnpdf and improving those I'll do that in a separate PR to leave this in this state as benchmark. I'll remove the global pars file so that instead most of the info will be read from runcards like the nnpdf ones (the format of global pars is similar anyway, I just want to avoid having the stateful configuration) |
|
One question @LucianHL, were you ever using the t0 covmat for sampling or only the experimental covmat? |
|
Thanks @scarlehoff sounds good.
I don't follow - the t0 covmat is used in the minimisation so I'm not sure what you mean. p.s. I'm on holiday this week so may not be super quick to respond but no problem to answer quick questions like that. |
|
By sampling I mean for the creation of the replicas (when doing
enjoy the holidays! And don't worry, I'm not managing to be quick to respond even when not on holidays... |
|
Ah ok, so |
|
Ah, ok. That's clear then.
No, but I'm isolating the calls to vp to avoid computing the same things many times and wasn't sure about that part. For now I'll leave it be. |
This is a first pass, my goal here will be to try and use directly calls to the vp API when possible. This (should) simplify a lot some parts, like the generation of the covmat, replicas etc (it just requires some care with the input).
For now this has required only one change in the NNPDF side: NNPDF/nnpdf#2268 in order to accept arbitrary objects that are not necessarily LHAPDF names (I put you as a reviewer there @tgiani). I think this is a nice addition and I think I'm happy with the current version since it allows the usage of the functions that @LucianHL already wrote with no changes.
I've also seen a few things that I could speed up in nnpdf which we only use for plots (since the fit code uses tensorflow) but that here are used for the fit.
@LucianHL for now I'm not removing anything but just duplicating and checking that things are numerically equal. I think I'd eventually need some help in preparing some config files because the combination of possible paths / options is not 100% clear to me yet.
For now the only change in behaviour is that the fakes PDF are saved to a temporary folder instead of the lhapdf data path since I don't always have write permissions to that folder.
I'll continue doing some changes in this branch. My short term goal here is to make the computation of the chi2 in just a few calls, which then I can optimize in the nnpdf side, while at the same time keeping everything flexible.