-
Notifications
You must be signed in to change notification settings - Fork 3
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
N3LO matching #83
N3LO matching #83
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #83 +/- ##
==========================================
Coverage 100.00% 100.00%
==========================================
Files 36 65 +29
Lines 2575 3060 +485
==========================================
+ Hits 2575 3060 +485
Flags with carried forward coverage won't be shown. Click here to find out more.
|
This reverts commit 107544a.
What are we missing to close this PR? |
I'm reviewing right now |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From now on, we should refrain to address multiple big tasks in a single PR:
- the
harmonics
branch should have not been merged in here, because it has been reviewed twice in this way - the
numba
cache enhancement was really needed, and even related to this PR, but it has been scattered all over, blowing up the number of files, so we should have done in a separate one (and merged immediately on develop, updating afterwards this, or otherwise merge this one before, and next apply thenumba
chore from a second PR, we havedevelop
branch exactly for this reason...)
P.S.: of course, I didn't check details and numbers of every single function, I checked tests and relying on them, but MMa numbers I had no way to check, just relying on you
gqq1p_cfca = -16*g3n + (132 - n*(340 + n*(655 + 51*n*(2 + n))))/(18.*np.power(n,2)*np.power(1 + n,2)) + (-14.666666666666666 + 8/n - 8/(1 + n))*S2 - (4*Sp2p)/(n + np.power(n,2)) + S1*(29.77777777777778 - 16/np.power(n,2) - 16*S2 + 8*Sp2p) + 2*Sp3p + 10*zeta3 + zeta2*(16*S1 - 16*Sp1p + 16*(1/n - np.log(2))) # pylint: disable=line-too-long | ||
gqq1p_cfcf = 32*g3n - (8 + n*(32 + n*(40 + 3*n*(3 + n)*(3 + np.power(n,2)))))/(2.*np.power(n,3)*np.power(1 + n,3)) + (12 - 8/n + 8/(1 + n))*S2 + S1*(40/np.power(n,2) - 8/np.power(1 + n,2) + 16*S2 - 16*Sp2p) + (8*Sp2p)/(n + np.power(n,2)) - 4*Sp3p - 20*zeta3 + zeta2*(-32*S1 + 32*Sp1p + 32*(-(1/n) + np.log(2))) # pylint: disable=line-too-long | ||
gqq1p_cfca = -16*g3n + (132 - n*(340 + n*(655 + 51*n*(2 + n))))/(18.*np.power(n,2)*np.power(1 + n,2)) + (-14.666666666666666 + 8/n - 8/(1 + n))*S2 - (4*Sp2p)/(n + np.power(n,2)) + S1*(29.77777777777778 - 16/np.power(n,2) - 16*S2 + 8*Sp2p) + 2*Sp3p + 10*zeta3 + zeta2*(16*S1 - 16*Sp1p + 16*(1/n - log2)) # pylint: disable=line-too-long | ||
gqq1p_cfcf = 32*g3n - (8 + n*(32 + n*(40 + 3*n*(3 + n)*(3 + np.power(n,2)))))/(2.*np.power(n,3)*np.power(1 + n,3)) + (12 - 8/n + 8/(1 + n))*S2 + S1*(40/np.power(n,2) - 8/np.power(1 + n,2) + 16*S2 - 16*Sp2p) + (8*Sp2p)/(n + np.power(n,2)) - 4*Sp3p - 20*zeta3 + zeta2*(-32*S1 + 32*Sp1p + 32*(-(1/n) + log2)) # pylint: disable=line-too-long |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, caching even np.log(2)
it is rather extreme, but fine of course...
# TODO: shall we use: 0.4 * 16.0 / (1.0 - logx) ? | ||
self.r = 0.4 * 16.0 / (-logx) | ||
# TODO: shall we use: 0.4 * 16.0 / ( - logx) ? | ||
self.r = 0.4 * 16.0 / (1.0 - logx) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess this has been tested, are the benefits documented somewhere? It might be useful for the future.
Consider writing something either here in the docstring, or in the Mellin page of the docs.
The only relevant point is documenting Mellin path choice #83 (comment), but we can even postpone and just merge, at this point we have to do it as soon as possible. |
I suggest to close and instead postpone the documentation to #116 |
I also updated dependencies with Poetry, so everything is alright. I leave up to @giacomomagni the task to resolve conflicts with |
Integrate the eko relevant features coming from #74 inside develop