Add LHAPDF parser to genpdf

[felixhekhorn]

I was about to write this thing for the third time and realized this was two times to many, so I decided to do it properly here. The new parser.py script is clean and easy reusable (which is the main purpose).

@evagroenendijk please take a review - I tried to preserve the old behaviour as much as I could, but still some small things changed here and there

@scarlehoff I know there is some cross talk between genpdf and nnpdf and I haven't double check yet what exactly you use on the other side, but maybe you know better or are quicker

I'm not sure on what to do with the benchmark files (benchmarks/ekobox/genpdf/) yet, if to fix them or to drop them. The correct thing would be to rewrite them so they don't use lhapdf (which should be possible) and make them executable in the CI again ...

TODO:

• fix benchmarks

[scarlehoff]

In nnpdf we use lhapdf as a library.

I did add a parser to lhapdf_management although the main point there was the metadata rather than the grids, in case you want to have a look https://github.com/scarlehoff/lhapdf_management/blob/main/src/lhapdf_management/pdfsets.py#L197

(the grids are also saved, but upon quick inspection there isn't really any new insight that can bring)

[felixhekhorn]

In nnpdf we use lhapdf as a library.

nono, I'm worried (correctly as I discuss in a second) about genpdf

I did add a parser to lhapdf_management although the main point there was the metadata rather than the grids, in case you want to have a look https://github.com/scarlehoff/lhapdf_management/blob/main/src/lhapdf_management/pdfsets.py#L197

(the grids are also saved, but upon quick inspection there isn't really any new insight that can bring)

My LhapdfDataBlock here is a more or less literal copy of your GridPDF there

The thing that worries me is this line and this one, which I break precisely here. So how to proceed? I can declare this a proper breaking change, which it is for nnpdf (although I'm not sure I would have put these function into our public scope, but that's a different discussion 🙈 ). The other less favoured option is to give up on this PR and copy the new parser into my other projects directly (which already depend on EKO, so this would have been convenient eventually). Also I'd say the code is arguably not the best in this specific part ...

PS: of course the best place for all this would be LHAPDF itself, but alas ...

[scarlehoff]

Why do you need to break it?

[evagroenendijk]

@felixhekhorn Sure, I will look at it either tomorrow or Monday! Just so that I have a broad idea, what was wrong and what are you trying to do?

[felixhekhorn]

Why do you need to break it?

Now, that I see the actual used code in nnpdf I think I can maybe rewrite it in such a ways that it would be backward compatible ... I can try ... it would be technically still different objects being passed around, but, thanks to Python, no one would notice

@felixhekhorn Sure, I will look at it either tomorrow or Monday! Just so that I have a broad idea, what was wrong and what are you trying to do?

nothing directly wrong, but what I want is a central place which provides me with tools for LHAPDF manipulations - and eko and in particular genpdf is a natural place for that. In particular, what I need in some other projects of mine is to write LHAPDF files, which are generated in some way. Two simple examples are adding LHAPDF files and charge conjugating LHAPDF files (and, yes, there is also the cross 🙃 )

[evagroenendijk]

Hi @felixhekhorn I looked at it and tested if it does not break anything in eko. In my opinion it makes sense the way you created the parser.py with this LhapdfDataBlock class, and it looks like most of the other changes are just implementing this in what was already there. I noticed only two small things in parser.py, I don't know if you want to do anything with it but thought I'd mention them anyway:

1. in line 34 you have

def add(self, other):

I guess that could be turned into

def add(self, other)->LhapdfDataBlock:

2. in line 27 you define this is_valid() but it looks like you're not using it anywhere yet if I'm not mistaken. I guess you could put it directly when you make a block, e.g. in line 91, no?

As I said, small things so maybe you prefer to leave it like this :)