Fix scale variations scheme B #125
Conversation
|
@andreab1997 @giacomomagni please have a look on what I wrote and concluded concerning the minus signs - do you agree? |
Codecov Report
@@ Coverage Diff @@
## develop #125 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 77 77
Lines 3541 3552 +11
=========================================
+ Hits 3541 3552 +11
Flags with carried forward coverage won't be shown. Click here to find out more.
|
|
@giacomomagni you correctly said here: https://github.com/N3PDF/eko/blob/039beec06aec5f14be82b696817bd6b5b4ed2b6f/src/eko/scale_variations/expanded.py#L170 the unit test is now failing ... I'm not entirely sure on what it should test ... also the non-commutativity in scheme B should now be taken care of ... but actually it seems the discrepency is worse now ... can you please have a look? |
felixhekhorn
added
enhancement
|
Hi @felixhekhorn, If I look at eq. 3.32 (scale variation exponentiated) you can see that the sign convention holds. While doing the work I annotated that there is also a minus sign in the definition of |
@andreab1997 could you please double check my updated version - @giacomomagni convinced me that he was right in the first place ... |
I'd recommend implementing the second point (about the starting scale), and once it is in place, check that scheme B and C are actually sufficiently compatible (or that expanded and exponentiated are compatible, using TRN) |
|
@felixhekhorn do you have a candidate now, or still investigating? |
Last time we talked, we noticed again that a change of a minus sign would lead to compatible results with scheme C. However, we checked together basically all the part of the code related to scale variations and we did not find where this minus sign could be wrong. I don't know if @felixhekhorn has news about this. |
|
Let's discuss this later ... (I really need to prepare the meeting later now 🙈 😱 ) |
|
Since we're about to close a bunch of PRs, let's close also this one (despite that there is no definite conclusion on scheme B) Changes should be:
this PR does not include the split of the SV kernel from the evolution kernel, but I suggest to postpone this to a separate PR |
|
@giacomomagni may I ask you to review this, please? This week I'll be pretty busy... |
@giacomomagni you're absolutely correct and I should apologize to have asked for a review before the thing was actually finished ... as you can see the merge has been actually non-trivial; please give me an explicit approval when you're happy (note that (as usual) we get the green tick only after the benchmarks finished) |
closes #121
most likely a missing minus sign from Eq. 3.33Originally posted by @felixhekhorn in #121 (comment)