Write down the versions for both eko and pineko #109
Conversation
| requires = ["poetry-core>=1.0.0"] | ||
| build-backend = "poetry.core.masonry.api" | ||
| requires = ["poetry-core>=1.0.0", "poetry-dynamic-versioning"] | ||
| build-backend = "poetry_dynamic_versioning.backend" |
There was a problem hiding this comment.
Ah, I actually lost that there was also a back-end support. Thanks @scarlehoff!
|
eko doesn't allow for an unknown key in the runcard, is there a "description" or any other free form field that I can use for the pineko version? |
|
Nope, there is not... But EKO should be fully reproducible given its runcards, including the EKO version. Why do you want the Pineko version inside the EKOs? |
|
pineko creates the runcard, so |
But not in the EKO. That information you could write in the theory. My point is that EKO is already decoupled from that: whatever version of Pineko created the runcard, if it's the same runcard it will be the same EKO (no matter Pineko). |
|
The problem is that in general you might have a theory made with different versions of pineko* and I would like to save that information. The theory, template and eko might be the same but I'd like the operator card to save the pineko version as well. *because of some ekos being very slow and, eg jets |
|
Add a log file to the theory? My point is that I'd like EKO and the EKOs to know nothing about Pineko, for better isolation. |
|
But then you need a log file per dataset, is that what you meant? However, I've done a small change so the information is stripped from the operator card before feeding it to eko. |
Not exactly: I meant a single log file, and you log to it adding lines every time (i.e. you cat the log files per-dataset into a single one). |
You mean, having a sort of a |
|
Actually, I don't think this is a good idea. I think that if there's no way of keeping the information of the pineko version in the operator card itself is better to drop it. I'd like to keep it though (just like the |
|
But the current state is quite useless, no? since the "pineko operator card", which is now different then the "eko operator card", is never saved in the FK and so this information is completely local ... what we should do is open eko at the time of convolution, extract the (eko-) op_card and dump into the metadata ... |
The convolution is also done by Pineko, so that is yet a further Pineko version. About the runcard I really believe there is no need to preserve this information through the EKO. And I would not account for the infinite amount of combinations on how you could generate the runcards: either you used a single version of Pineko for the whole process, or we should add some more free form logs, as the log file proposed above. Keeping structured information is not much more helpful for reproducibility, since you then should propagate it consistently and the propagation process itself might change with the Pineko version... |
Indeed! I don't want to preserve the information beyond the operator card itself. If you look at the changes in the code, you can see that I drop it. The point is that the operator card is a product of pineko, and as a product it should be possible to reproduce it and for that we need to know the pineko version. The eko is a product of The |
|
In the case of the eko op_card, I think this is actually useful information - also in the light of sim-FONLL we want to see the grids are convoluted with the correct FNS ... so I still suggest to add below this line Line 243 in 3310f43 fktable.set_key_value("eko_operator_card", operators.operator_card)
|
|
But this issue deals with the operator card, not with the fktable itself. |
ok accepted, my comment is out of scope - still I'm not happy with this, because this creates a new configuration file which is specific to pineko: you can not use these cards any longer in regular eko, i.e. without going through the pineko interface |
Yes, this is why I was asking for a |
we currently use this https://github.com/NNPDF/eko/blob/a8ee0a8c90462bfa83a56bcaf3855b7fae461b38/src/eko/io/dictlike.py#L36 |
True, but this is happening on top of the dictionary. And the dictionary is actually loaded from YAML, so comments are actually automatically lost. |
| @@ -153,6 +153,8 @@ def write_operator_card(pineappl_grid, default_card, card_path, tcard): | |||
|
|
|||
| with open(card_path, "w", encoding="UTF-8") as f: | |||
| yaml.safe_dump(operators_card, f) | |||
| f.write(f"# {pineko_version=}") | |||
There was a problem hiding this comment.
I guess this was not what you wanted to write, but:
| f.write(f"# {pineko_version=}") | |
| f.write(f"# pineko_version={pineko_version}") |
(mainly because of the =, that makes little sense without : and a width)
There was a problem hiding this comment.
{pineko_version=} is shorthand for pineko_version={pineko_version}
But I can do pineko_version: {pineko_version} instead if you prefer the : (it's a comment so it doesn't really matter how is it written)
There was a problem hiding this comment.
Ah, great. I forgot about this option.
The reference to the : was only because = can be used as a format specifier, but the specifiers should be separated by the identifier with the :.
It is fine as it is.
There was a problem hiding this comment.
Btw, it was new in py3.8, that by now it is perfectly fine (py3.7 officially died one month ago).
https://docs.python.org/3/whatsnew/3.8.html#bpo-36817-whatsnew
Closes #108