Move permanent part of fkutils here

[felixhekhorn]

This PR should have really started at 6d93747

• log time taken for each expensive operation

[alecandido]

Improve CLI, please :)

[felixhekhorn]

I wonder whether to change here:
https://github.com/N3PDF/pineko/blob/784dee8e4a016fc033461fe9ac4e21d1ead67294/src/pineko/configs.py#L79-L83

and add a else after the if, because otherwise you get something like this:

$ pineko -c /home/a.toml theory opcards 213 LHCB_Z_13TEV_DIELECTRON
Configurations loaded from '/home/a.toml'
Analyze LHCB_Z_13TEV_DIELECTRON
Success: Wrote card with 1 Q2 points to 
/home/felix/Physik/N3PDF/FK/pineko/data/operator_cards/LHCB_DY_13TEV_DIELECTRON.yaml

where /home/a.toml never existed and the config truely used the default one

[alecandido]

Don't change detect: if you don't like this behavior, you should change load

https://github.com/N3PDF/pineko/blob/784dee8e4a016fc033461fe9ac4e21d1ead67294/src/pineko/configs.py#L101-L102

or even change where it's used:

https://github.com/N3PDF/pineko/blob/784dee8e4a016fc033461fe9ac4e21d1ead67294/src/pineko/cli/_base.py#L20-L24

Indeed, detect is already passing all the information: since it no one existed, it's even raising an error, that is later caught by load. So, detect has done its job.

I'd suggest not to call detect directly inside load, but to pass the detect result as load argument.
Consequently, the exception management should not be done inside load, but inside command, and so you can decide to print a different message, or even to just fail, and keep propagating the exception.

[felixhekhorn]

I attempted an improvement on configs in 3a53e56 , but there are still things with which I'm not happy:

• what is add_scope supposed to do?
• with the changes there, the script at least is no longer lying, i.e. it is telling the file it really used - but it is only doing so if one explicitly used the -c flag; that seems inconsistent ...
• furthermore the -c flag is not doing, what is promising: "Explicitly specify config file (it has to be a valid TOML file)."
  • first, it does not need to be a file, also a path would go through
  • and it should really do what it says, which it isn't: i.e. if you specify this flag you should read exactly that file (which would correspond to the else I mentioned) - the current implementation is rather adding another possibility
• I dropped the return {"paths": {"root": pathlib.Path.cwd()}} because it would result in a ValueError right after as I have some mandatory configs
• (also (now called) enhance_paths was inconsistent as it was modifying inplace and returning ...)
• (finally I also added more docs)

[alecandido]

• what is add_scope supposed to do?

It's a function I'm using in runcardsrunner corresponding instance, to add defaults for an entire section, but preserving values that have explicitly been specified.

(which would correspond to the else I mentioned)

This is false: the else would correspond to the case in which no path is specified. In that case either you return an error or you adopt some defaults. What is happening there is that you attempt a few sensible defaults, and if even those are not working, then raise.

• first, it does not need to be a file, also a path would go through

What's the difference between a path and a file? On CLI you can only write strings, i.e. paths...

• I dropped the return {"paths": {"root": pathlib.Path.cwd()}} because it would result in a ValueError right after as I have some mandatory configs

This is fine, I've defaults for everything in runcardsrunner, but it's more of a choice.

• (finally I also added more docs)

Always a good idea.

[felixhekhorn]

@alecandido @scarlehoff @cschwan @andreab1997

Dear all,

• I now believe to have all elementary stuff needed to build FK tables moved from https://github.com/NNPDF/fktables/blob/main/fkutils/builder.py to here, please take a look if you can and provide me with some feedback
• a tentative "manual" is given in README.md
• some improvements (@andreab1997 ) over the old implementation:
  • logs are now generated for every run (no more piping needed python build.py > logs/eko/bla.log)
  • grids can be inherited by command (instead of manual linking)
  • ekos can be inherited
  • no more prefixing with theory ID (so @scarlehoff you will be able to just copy the folder as is)
  • no more dependency on apfelcomb database but only ymldb (@scarlehoff I'm only reading the members for the rest I don't care)
  • no more special treatment of APPL thanks to ymldb
• some problems still to solve (can be done here or in a seperate PR):
  • allow scale variations
  • add argument to parser.get_yaml_information to allow only to ask for the name, so we can run eko without the actual grids being present
  • the hacks I needed for jets (i.e. https://github.com/NNPDF/fktables/blob/main/split_grid.py https://github.com/NNPDF/fktables/blob/main/jets_build.sh https://github.com/NNPDF/fktables/blob/86927f9460f8bbaf7f4daab0789c2d32283d3f2d/fkutils/builder.py#L76 https://github.com/NNPDF/fktables/blob/86927f9460f8bbaf7f4daab0789c2d32283d3f2d/fkutils/builder.py#L173 ) I did not port, but I hope Composite Output eko#105 will be available soon
  • I'd be happy to have some more opinions on Move permanent part of fkutils here #12 (comment)
• once we're happy there is a final step, i.e. moving the files from the old infrastructure to the new one - which mainly consists in stripping the prefixed theory ID and then to adjust the compare script to the new layout ...

[cschwan]

@felixhekhorn cheers, I already had a quick look at the README, see my comments and edits there.

[felixhekhorn]

@enocera of course I should have tagged you as well - apologies ;-) please feel free to take a look

[alecandido]

I still have to dig pineko.evolve and pineko.theory, but everything else it's good enough.

I left a few comments, mainly about code I suppose to be not needed, and I suggest to just drop.

[scarlehoff]

Once this is merged and a package created I can try and use it in vp.

[scarlehoff]

I'm going to merge this if nobody complains.

[felixhekhorn]

I opened #13 and #14 for the remaining problems (SV is tracked in #6)

[scarlehoff]

Then I can merge, right? As soon as we merge we can tackle the different issues one by one.

[felixhekhorn]

For future reference: a bin-by-bin-evolution (as needed for jets) is still possible if needed after NNPDF/eko#105 , but this time I would rather hack outside pineko then inside: we could define a bunch of fake-datasets: CMS_1JET_8TEV-bin00000 containing exactly one member which itself contains exactly one bin /cc @cschwan

[cschwan]

@felixhekhorn would it make sense to support this as a 'mode' in Pineko? It would exactly do what you descibe: 1) chop the grid into single bins, 2) evolve each bin and finally 3) merge all evolved bins together again.