Skip to content

Misc validphys speed ups#2320

Closed
scarlehoff wants to merge 4 commits intomasterfrom
speed_up_convolutions
Closed

Misc validphys speed ups#2320
scarlehoff wants to merge 4 commits intomasterfrom
speed_up_convolutions

Conversation

@scarlehoff
Copy link
Member

Trying to speed up a few things I noticed the convolutions spent a good fraction of the time just moving things around inside the dataframe.

Now, this is necessary because the old fktables didn't follow any particular order... when loading fktables with pineappl instead this is not the case. Among other things because all fktables get casted to the same array:

for x_point in missing_x_points:

with these changes a full vp-comparefit goes from 40 min to about 20 in my computer.

Note: I have not bothered to port this speed up to old fktables. In principle it could be done just the same by ordering them sensibly after they are loaded.

Note 2: we load the pineappl fktables into an intermediate dataframe that with these changes is no longer necessary, but that's trickier to remove since the fktable is assumed to be a dataframe at various points.

@scarlehoff scarlehoff requested a review from tgiani May 20, 2025 13:43
@scarlehoff scarlehoff added the run-fit-bot Starts fit bot from a PR. label May 20, 2025
@scarlehoff scarlehoff mentioned this pull request May 20, 2025
@github-actions
Copy link

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 😉!

@scarlehoff scarlehoff removed the run-fit-bot Starts fit bot from a PR. label May 21, 2025
@scarlehoff scarlehoff force-pushed the speed_up_convolutions branch from 5961d4b to 51500dd Compare May 21, 2025 07:04
@scarlehoff scarlehoff changed the base branch from master to data_update_250521 May 21, 2025 07:05
Base automatically changed from data_update_250521 to master May 21, 2025 08:03
@scarlehoff scarlehoff force-pushed the speed_up_convolutions branch from 51500dd to cb68d61 Compare June 3, 2025 12:48
@scarlehoff
Copy link
Member Author

Hi @tgiani, thanks for looking into this. I've added a bunch of comments and reordered a bit the code so that it is clearer what is done for old fktables and for new

@scarlehoff scarlehoff force-pushed the speed_up_convolutions branch from cb68d61 to d51b3b6 Compare June 10, 2025 08:14
Radonirinaunimi added a commit that referenced this pull request Jun 17, 2025
Radonirinaunimi added a commit that referenced this pull request Jun 17, 2025
@scarlehoff
Copy link
Member Author

All commits from this branch are now in #2317 (I won't delete this branch for now in case we need to split it again, but let me close the PR for the time being)

@scarlehoff scarlehoff closed this Jun 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant