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

Fix LHA VFNS SV benchmark #218

Closed
wants to merge 1 commit into from
Closed

Conversation

felixhekhorn
Copy link
Contributor

@felixhekhorn felixhekhorn commented Feb 24, 2023

Closes #215

The fix here mirrors the original behaviour in

eko/src/eko/couplings.py

Lines 445 to 453 in d32b95a

# adjust factorization scale / renormalization scale
fact_to_ren = theory_card["fact_to_ren_scale_ratio"]
heavy_flavors = "cbt"
if masses is None:
masses = np.power(
[theory_card[f"m{q}"] / fact_to_ren for q in heavy_flavors], 2
)
else:
masses = masses / fact_to_ren**2

I wonder whether we need to guard this division by scheme A ...

@felixhekhorn felixhekhorn added the bug Something isn't working label Feb 24, 2023
@@ -92,7 +92,7 @@ def __init__(
couplings=new_theory.couplings,
order=new_theory.order,
method=couplings_mod_ev(new_operator.configs.evolution_method),
masses=masses,
masses=np.array(masses) / new_theory.xif**2,
Copy link
Member

Choose a reason for hiding this comment

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

This is changing everything, and in particular also the MHOU computed by @andreab1997

In principle, if it was a bug is a good thing. But why do you believe this should be so general?
I'm not sure why, if I change the factorization scale, the quark masses values should change in the $\alpha_s$ running...

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes this is really strange. Moreover you are rescaling them by xif squared but the masses seem to be linear. If this fixes the benchmark, it suggestes a bug somewhere else...

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think masses are squared as well

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Indeed they are:

masses = [mq.value**2 for mq in new_theory.quark_masses]

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note that this has indeed no impact in FFNS, as masses are there either 0 or $\infty$, and thus is not discussed by our scheme B check

Copy link
Collaborator

@giacomomagni giacomomagni Feb 24, 2023

Choose a reason for hiding this comment

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

This is also consistent with what we have in benchmarks:

masses=(threshold_list / fact_to_ren_lin**2).tolist(),

the other place where Couplings is called is in msbar_masses.py , but since SV with \bar{MS} masses are not supported, that should be it.

Copy link
Contributor

Choose a reason for hiding this comment

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

Discussing with @felixhekhorn we convinced ourselves that whatever we do for alpha_s in scheme A we should do exactly the same in scheme B. In fact, there is no reason to do different things for the two schemes in that they are doing basically the same evolution (the only difference being that scheme A is using the varied splitting functions and scheme B is not). Note that (according to @felixhekhorn ) in eko==0.10 all the masses were shifted in this way, including also scheme B. In particular, in scheme B we have as1 that is used (correctly) for both the evolution and the "additional sv term" and we actually need to compute it rescaling the masses (as we were doing before). So, according to us, scheme B in eko==0.12 is now bugged. Do you agree?

Copy link
Contributor

Choose a reason for hiding this comment

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

Of course we still need to understand why we should rescale the masses but this is not something new. My current hypothesis is that, since the masses always enter as ratio like m^2 / mu^2, dividing m^2 / xif ^2 is actually equivalent to multiply mu^2 * xif^2 = mu_f ^2 which in fact is the factorization scale.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes I agree on both counts

@giacomomagni
Copy link
Collaborator

@felixhekhorn, should we remove all the fact_to_ren_ratio still around before merging this?

@alecandido
Copy link
Member

If this is recognized as a clear bug, we can merge this as a bug fix also without a clear reason, but consistently for any scheme (and if possible removing all fac_to_ren and related).

However, in case open a further dedicated issue, please. Identifying the reason is extremely relevant to be sure we are acting correctly and consistently.

@andreab1997
Copy link
Contributor

If this is recognized as a clear bug, we can merge this as a bug fix also without a clear reason, but consistently for any scheme (and if possible removing all fac_to_ren and related).

However, in case open a further dedicated issue, please. Identifying the reason is extremely relevant to be sure we are acting correctly and consistently.

About this, @felixhekhorn Have you tried running the test rescaling the scales (so multiplying them by xif^2) instead of rescaling the masses? If the results turn out to be the same (or still compatible with the benchmark) I would say that what I said in the previous comment is correct and it is a resonable fix (and we should propagate immediately to scheme B).

@felixhekhorn
Copy link
Contributor Author

Close in favor of #222

@felixhekhorn felixhekhorn deleted the fix-couplings-masses branch March 9, 2023 16:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

LHA VFNS SV is broken
4 participants