Skip to content

Update PineAPPL version to v1#2217

Merged
Radonirinaunimi merged 13 commits into
masterfrom
update-pineappl-v1
Jun 12, 2025
Merged

Update PineAPPL version to v1#2217
Radonirinaunimi merged 13 commits into
masterfrom
update-pineappl-v1

Conversation

@Radonirinaunimi
Copy link
Copy Markdown
Member

For the time being, this is primarily needed because pineko (NNPDF/pineko#206) depends on it. This does not introduce physic's changes yet, ie still does not allow for an arbitrary number of convolutions.

@Radonirinaunimi Radonirinaunimi marked this pull request as draft November 18, 2024 16:51
@scarlehoff
Copy link
Copy Markdown
Member

sorry, wouldn't it be better to instead of having a branch here just for the pineko dev branch, to remove for now nnpdf from the pineko dev branch?

Or do you need nnpdf there?

@Radonirinaunimi
Copy link
Copy Markdown
Member Author

sorry, wouldn't it be better to instead of having a branch here just for the pineko dev branch, to remove for now nnpdf from the pineko dev branch?

Or do you need nnpdf there?

Yes, nnpdf would be needed there, not only for some tests (which I'd like all of them to pass) but also to fully test that the evolution pipeline is working perfect. But also, as soon as v1 is available, we'll need to upgrade asap anyway in order to get rid of the reliance on the PineAPPL metadata (and ofc a bit later on to generalize to arbitrary convolution).

Copy link
Copy Markdown
Member

@scarlehoff scarlehoff left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok then! I've left some small comments, although it is WIP I thought better to mention these small points sooner rather than later.

Just one question, in principle old fktables we will always be able to read, right?

Comment thread validphys2/src/validphys/coredata.py Outdated
Comment thread validphys2/src/validphys/pineparser.py
Comment thread validphys2/src/validphys/pineparser.py
@Radonirinaunimi
Copy link
Copy Markdown
Member Author

ok then! I've left some small comments, although it is WIP I thought better to mention these small points sooner rather than later.

Thanks for the comments @scarlehoff!

Just one question, in principle old fktables we will always be able to read, right?

Yes, old FK tables will still be read as before perfectly. The only difference is that now when PineAPPL reads these FK tables it'll add onto them the new attributes (for e.g. convolutions) .

@Radonirinaunimi Radonirinaunimi added the waiting-pineappl-v1 Waiting for the release of PineAPPL v1 label Nov 27, 2024
@Radonirinaunimi Radonirinaunimi marked this pull request as ready for review February 18, 2025 23:50
@scarlehoff
Copy link
Copy Markdown
Member

As of 1.0.0a4 there's a problem with fktables that include bins that are 0. It is possible to still fits simply by removing these 0 points as they are also ignored by the nnpdf code:

#!/usr/bin/bash

for i in *.lz4
do
    if ! python -c "import pineappl ; pineappl.fk_table.FkTable.read(\"${i}\").table()" 2>/dev/null
    then
        echo "Removing extra points from" $i
        bins=$(pineappl read ${i} --bins | awk 'END{print $1}')
        pineappl write ${i} --delete-bins 1-${bins} tmp.lz4
        mv tmp.lz4 ${i}
    fi
done

@scarlehoff
Copy link
Copy Markdown
Member

This needs to be rebased to ensure that everything works.

I would say let's ignore here the fact that pineappl>=1.0 is not compatible with eko>=0.15 since they never need to talk for this repo (it is only a problem for pineko, but pineko depends only on nnpdf-data not in the full nnpdf)

@cschwan
Copy link
Copy Markdown
Contributor

cschwan commented May 20, 2025

How is PineAPPL incompatible with EKO?

@scarlehoff
Copy link
Copy Markdown
Member

It was mentioned in the last code meeting, but I don't know the details.

@achiefa
Copy link
Copy Markdown
Contributor

achiefa commented May 23, 2025

Probably that's off-topic, but I just wanted to mention that with pineappl v1, we need to update the API extension notebook too. I was thinking that this branch might be the most appropriate place to update it. I can take care of that if you don't mind.

@Radonirinaunimi
Copy link
Copy Markdown
Member Author

Probably that's off-topic, but I just wanted to mention that with pineappl v1, we need to update the API extension notebook too. I was thinking that this branch might be the most appropriate place to update it. I can take care of that if you don't mind.

Yes, sure.

PS: There isn't a conda version of v1 yet so for the time being please just ignore the failing tests.

@scarlehoff scarlehoff added the run-fit-bot Starts fit bot from a PR. label Jun 11, 2025
@scarlehoff
Copy link
Copy Markdown
Member

If you could update this as well https://github.com/NNPDF/nnpdf/blob/master/validphys2/examples/API_extension_Pineappl.ipynb before merging that would be great. Otherwise I can have a go tomorrow or the day after and merge it then.

@github-actions
Copy link
Copy Markdown

Greetings from your nice fit 🤖 !
I have good news for you, I just finished my tasks:

Check the report carefully, and please buy me a ☕ , or better, a GPU 😉!

Copy link
Copy Markdown
Member

@scarlehoff scarlehoff left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do the honors

@Radonirinaunimi Radonirinaunimi merged commit bf9a8ec into master Jun 12, 2025
11 checks passed
@Radonirinaunimi Radonirinaunimi deleted the update-pineappl-v1 branch June 12, 2025 07:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dependencies run-fit-bot Starts fit bot from a PR. waiting-pineappl-v1 Waiting for the release of PineAPPL v1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants