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

Adding FHMRUVV N3LO splitting functions #335

Merged
merged 35 commits into from
Jan 12, 2024
Merged

Adding FHMRUVV N3LO splitting functions #335

merged 35 commits into from
Jan 12, 2024

Conversation

giacomomagni
Copy link
Collaborator

@giacomomagni giacomomagni added enhancement New feature or request physics new physics features benchmarks Benchmark (or infrastructure) related labels Dec 27, 2023
Copy link
Contributor

@felixhekhorn felixhekhorn left a comment

Choose a reason for hiding this comment

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

mostly minor stuff, but

  • please check explicitly that you didn't break the LHA benchmarks since they were broken before your last commit and I'm not sure that fixes it (as it was numba complaining)
  • we should remember to point them (=FHMRUVV) explicitly to this PR and tell them that they can (and should) start playing on their own

@felixhekhorn felixhekhorn changed the title Adding Moch et al N3LO splitting functions Adding FHMRUVV N3LO splitting functions Jan 10, 2024
@giacomomagni
Copy link
Collaborator Author

Do we want to have to have use_fhmruvv also in the QED kernels?

@felixhekhorn
Copy link
Contributor

Do we want to have to have use_fhmruvv also in the QED kernels?

is the effort manageable? if it is too much work, I'd say this is surely no priority ... if it is doable, we have to explicitly mention it when writing the mail ...

@giacomomagni
Copy link
Collaborator Author

giacomomagni commented Jan 10, 2024

is the effort manageable? if it is too much work, I'd say this is surely no priority ... if it is doable, we have to explicitly mention it when writing the mail ...

It's just adding few if statements, I can do it now.

EDIT: done in 517ae10

Copy link
Contributor

@felixhekhorn felixhekhorn left a comment

Choose a reason for hiding this comment

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

The moment https://github.com/NNPDF/eko/actions/runs/7489901631/job/20387536993 finished and you checked it, we can merge

@felixhekhorn
Copy link
Contributor

Numba failed in https://github.com/NNPDF/eko/actions/runs/7489901631 ... but locally running poe lha -m "nnlo and sv and ffns" is just fine ...

I should have almost the same python (3.10.13 (me) vs. 3.10.10 (github)), the same numba (0.57.1), and the same numpy (1.24.4); ocard hash is the same (8beae75) (though theory not, but this was already discussed)

any ideas? @alecandido maybe?

@giacomomagni
Copy link
Collaborator Author

giacomomagni commented Jan 11, 2024

Numba failed in https://github.com/NNPDF/eko/actions/runs/7489901631 ... but locally running poe lha -m "nnlo and sv and ffns" is just fine ...

I should have almost the same python (3.10.13 (me) vs. 3.10.10 (github)), the same numba (0.57.1), and the same numpy (1.24.4); ocard hash is the same (8beae75) (though theory not, but this was already discussed)

any ideas? @alecandido maybe?

It might be due to banana!!
before n3lo_ad_variations had only 4 elements, now 7 can be required!

@giacomomagni
Copy link
Collaborator Author

Okay LHA benchmarks are now working again. 12918f9
Although the proper fix would be a new banana version...

@felixhekhorn
Copy link
Contributor

Okay LHA benchmarks are now working again. 12918f9 Although the proper fix would be a new banana version...

good catch! let's merge if https://github.com/NNPDF/eko/actions/runs/7500258647/job/20418576973 goes through so we can move on

@niclaurenti niclaurenti merged commit 5f16a8d into master Jan 12, 2024
6 checks passed
@felixhekhorn felixhekhorn deleted the fhmv_n3lo_ad branch January 12, 2024 10:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
benchmarks Benchmark (or infrastructure) related enhancement New feature or request physics new physics features
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants