Add neopdf PDF interpolation backend#2478
Conversation
|
Thanks for this. My main comment is that the validphys is already provider agnostic and the backend selecting code already exists: https://github.com/NNPDF/nnpdf/blob/master/validphys2/src/validphys/lhapdf_compatibility.py I think what you did should be easy to modify so that it works similarly to pdfflow, without breaking it or adding a third way of changing the backend. As to how to change the backend, I would put it in the profile. With pdfflow is just a "in case of failure finding lhapdf". Then we can eventually change the default. (the conda dependence will be there 4ever for pandoc reasons I'm afraid :__) |
| for member in self.members: | ||
| vals = np.array(member.xfxQ2s(pids, xgrid, q2grid)) | ||
| results.append(vals.reshape(nfl, nx, nq)) | ||
| return np.array(results) |
There was a problem hiding this comment.
This is repeating actually a lot of codes (which I'm strongly against)
As far as I can see you can do the same I did for pdfflow but the wrapper would be just 4 lines or so instead of an entire class.
Then the make_pdf in pdf_compatibility is the only place in which a choice needs to be made.
Most importantly, core.py should have no changes.
There was a problem hiding this comment.
Thanks @scarlehoff for having looked into this already. This is still very much in WIP (although things are already in place and working) and thus I am not entirely convinced yet about how to best implement the interface. However, I fully agree with you about the redundancy part and started addressing this in 484110. Now, the unnecessary modules/codes have been removed.
As to how to change the backend, I would put it in the profile. With pdfflow is just a "in case of failure finding lhapdf". Then we can eventually change the default.
Yes, I agree with this.
(the conda dependence will be there 4ever for pandoc reasons I'm afraid :__)
Yeah, which we may want to get rid of at some point... although the alternative is not so clear.
| docs = ["sphinxcontrib-bibtex", "sphinx-rtd-theme", "sphinx", "tabulate"] | ||
| qed = ["fiatlux"] | ||
| nolha = ["pdfflow", "lhapdf-management"] | ||
| neopdf = ["neopdf-hep"] |
There was a problem hiding this comment.
| neopdf = ["neopdf-hep"] | |
| neopdf = ["neopdf-hep", "lhapdf-management"] |
I haven't maintained pdfflow in a while and you are actively maintaining neopdf. Perhaps it makes sense to change nolha to pdfflow and make neopdf be nolha.
| """ | ||
| backend = os.environ.get(_BACKEND_ENV_VAR, "lhapdf").lower() | ||
|
|
||
| if backend not in _VALID_BACKENDS: |
There was a problem hiding this comment.
fwiw, to get the information from the profile just get_nnpdf_profile() is enough (from nnpdf_data::utils)
I'm not sure right now whether one can add variables at will without changing that function (in any case it would be trivial). The important thing is, don't add a new default to get_nnpdf_profile for pdf_backend but rather add explicitly to validphys/nnprofile_default.yaml pdf_backend: lhapdf.
The goal I have in mind is that people can start using neopdf there if they like and in a few months we try to swap the default and see what happens.
Radonirinaunimi
force-pushed
the
neopdf-backend
branch
from
48cb2e7 to
f2cda38
Compare
2 participants
The following introduces a PDF backend module so that one can in principle support any PDF interpolation library. Currently, LHAPDF and NeoPDF are supported and they can be used interchangeably given that they are numerically equivalent (bit-for-bit). (This eventually paves the way to a Conda-free dependency).