Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rotate the eko back to save disk space #117

Merged
merged 6 commits into from
Sep 20, 2023
Merged

Rotate the eko back to save disk space #117

merged 6 commits into from
Sep 20, 2023

Conversation

andreab1997
Copy link
Contributor

We want to rotate the eko back to the usual grid of 50 points to save disk space. In fact, as noticed by @giacomomagni, depending on the dataset, there is a factor up to ~30 in size between the original eko and the eko after the rotation (for the jets dataset the factor is ~24).

@alecandido
Copy link
Member

Please, avoid rotating back. Just keep the original one (copy/duplicate it before).

Otherwise, you will accumulate more and more numerical error.

@andreab1997
Copy link
Contributor Author

Please, avoid rotating back. Just keep the original one (copy/duplicate it before).

Otherwise, you will accumulate more and more numerical error.

Yes this was exactly my point. However @giacomomagni pointed out that copying 60gb every time (for the jets ekos) is a bit extreme.

@alecandido
Copy link
Member

Yes this was exactly my point. However @giacomomagni pointed out that copying 60gb every time (for the jets ekos) is a bit extreme.

If you do not need the original one, then you can well thrash it, and save space. But if you need it, then instead you should keep.
Notice that there is less computation involved in the copy, rather than going back and forth, so it's not a problem of performances. If it's a problem of disk space, you cancel the rotated EKOs right after. If you have enough space, and you want to save computation, you keep both.

The only case which we can not account for is if you do not have enough space to even perform the operation (copying), and you want to keep the original EKO.
In this case, the optimal solution would be to rotate on the fly, so we should give back an iterator from manipulate, that for each value of the scale: loads the corresponding array, rotates it in memory, returns the result without saving it. I.e., if this is a concern, we need a lazy iterator.

@felixhekhorn
Copy link
Contributor

In this case, the optimal solution would be to rotate on the fly, so we should give back an iterator from manipulate, that for each value of the scale: loads the corresponding array, rotates it in memory, returns the result without saving it. I.e., if this is a concern, we need a lazy iterator.

Maybe we should start rolling out eko v0.14 with NNPDF/eko#295 and specifically NNPDF/eko#292

@giacomomagni
Copy link
Contributor

I'm try to see if this temporary 5016b86 solution is doable also for jets.

Copy link
Contributor

@giacomomagni giacomomagni left a comment

Choose a reason for hiding this comment

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

Thanks @andreab1997 this solves the issue.
I'd be happy to merge.

src/pineko/theory.py Outdated Show resolved Hide resolved
src/pineko/theory.py Outdated Show resolved Hide resolved
@andreab1997
Copy link
Contributor Author

Thanks @andreab1997 this solves the issue. I'd be happy to merge.

I would be happy to merge as well now.

@giacomomagni giacomomagni merged commit 12a351b into main Sep 20, 2023
5 checks passed
@giacomomagni giacomomagni deleted the rerotate branch March 15, 2024 13:29
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.

4 participants