Fix backward matching

[niclaurenti]

Pull request made to fix the bug in the backward matching:
in the function build_ome the argument backward_method arrives as an object of the type InversionMethod.EXACT or InversionMethod.EXPANDED but we are checking it to be equal to strings. In conclusion the backward matching is never applied (the matching operator is always the upwards one) unless we add the .value.

[felixhekhorn]

• as conjectured: this most likely is another side effect of Use tempdir during execution #172 (since the thing was introduced there)
• can you please fix the tests? (it seems they are not using what is used in practice)
• I wonder whether to fix in the opposite way, i.e. fixing the RHS instead of LHS: if backward_method.value == InversionMethod.EXACT (constant literals should be available inside nuba, see

  eko/src/eko/evolution_operator/__init__.py

  Line 409 in 9538753

  if sv_mode == sv.Modes.exponentiated:

  )
• minor comment (not to be fixed in this PR): we should kill strings from numba since they are slowing down
• I immediately triggered the LHA benchmark https://github.com/NNPDF/eko/actions/runs/4755747561

[felixhekhorn]

Also should we add a forward-backward run to the iso-bench? actually, can we bring

eko/benchmarks/eko/benchmark_evol_to_unity.py

Lines 109 to 112 in 9538753

	# def test_matching_grid(
	# self,
	# ):
	# # test matching_ome_forward @ matching_ome_backward gives ID matrix or zeros

back to live?

[felixhekhorn]

Also should we add a forward-backward run to the iso-bench? actually, can we bring

eko/benchmarks/eko/benchmark_evol_to_unity.py

Lines 109 to 112 in 9538753

	# def test_matching_grid(
	# self,
	# ):
	# # test matching_ome_forward @ matching_ome_backward gives ID matrix or zeros

back to live?

maybe the problem was time ... @giacomomagni do you remember?

[alecandido]

@felixhekhorn should we merge the quick fix, and worry about tests at a later stage?

[niclaurenti]

@felixhekhorn should we merge the quick fix, and worry about tests at a later stage?

Before merging I would like to perform the benchmark of evolving to 1.65 and then back to 100 to see whether it is fixed or not

[giacomomagni]

Also should we add a forward-backward run to the iso-bench? actually, can we bring

eko/benchmarks/eko/benchmark_evol_to_unity.py

Lines 109 to 112 in 9538753

	# def test_matching_grid(
	# self,
	# ):
	# # test matching_ome_forward @ matching_ome_backward gives ID matrix or zeros

back to live?

maybe the problem was time ... @giacomomagni do you remember?

If I recall correctly problems where both time but also accuracy, because you had to test against and identity matrix, which was not really good.

[giacomomagni]

Some minor comments.
Could you please update also here:

eko/src/eko/evolution_operator/operator_matrix_element.py

Line 34 in 9538753

backward_method : ["exact", "expanded" or ""]

eko/src/eko/evolution_operator/operator_matrix_element.py

Line 117 in 9538753

backward_method : ["exact", "expanded" or ""]

and maybe we should use None instead of "" here and similar:

eko/src/eko/evolution_operator/operator_matrix_element.py

Line 143 in 9538753

(backward_method != "" and ker_base.is_singlet)

Otherwise also adding .value in config would be more consistent with the other modules of Eko. Simply changing here:

eko/src/eko/evolution_operator/grid.py

Line 86 in 9538753

config["backward_inversion"] = configs.inversion_method

[alecandido]

Otherwise also adding .value in config would be more consistent with the other modules of Eko. Simply changing here:

config is part of the issue: if it hadn't passed through an opaque dictionary, we might have been warned of the type inconsistency by some static analyzer (I will soon add mypy explicitly, but PyRight in the editor should have already noticed...). I would keep propagating the more specific types, in order to avoid similar issues later on.

[niclaurenti]

Hi @giacomomagni

and maybe we should use None instead of "" here and similar:

I agree with using None instead of ""

Otherwise also adding .value in config would be more consistent with the other modules of Eko. Simply changing here:

but I would pass the whole object to config and not just the string

[alecandido]

Actually, thinking about the alternatives "" and None for backward_inversion, I believe we can avoid both: let's always have an inversion strategy, this will avoid us to always consider the case in which it is not there.

If we need a default we could pick one of the two (in case we really want a default, and we really want to avoid one of them, we can add a NULL variant to InversionMethod, but I'd prefer to avoid).

@felixhekhorn

[giacomomagni]

Actually, thinking about the alternatives "" and None for backward_inversion, I believe we can avoid both: let's always have an inversion strategy, this will avoid us to always consider the case in which it is not there.

I think the None option was done in order to avoid passing an additional variable is_backward to quad_ker or similar.
But for sure there are other options.

[felixhekhorn]

Actually, thinking about the alternatives "" and None for backward_inversion, I believe we can avoid both: let's always have an inversion strategy, this will avoid us to always consider the case in which it is not there.

If we need a default we could pick one of the two (in case we really want a default, and we really want to avoid one of them, we can add a NULL variant to InversionMethod, but I'd prefer to avoid).

@felixhekhorn

as already said - most likely we can not stick to our no-default-strategy in the light of #145 ... so let's assume something

I think the None option was done in other to avoid passing an additional variable is_backward to quad_ker or similar. But for sure there are other options.

indeed, now as you say I remember:

eko/src/eko/evolution_operator/operator_matrix_element.py

Lines 52 to 70 in 9538753

	if backward_method == "expanded":
	# expended inverse
	if matching_order[0] >= 1:
	ome -= a_s * A[0]
	if matching_order[0] >= 2:
	ome += a_s**2 * (-A[1] + A[0] @ A[0])
	if matching_order[0] >= 3:
	ome += a_s**3 * (-A[2] + A[0] @ A[1] + A[1] @ A[0] - A[0] @ A[0] @ A[0])
	else:
	# forward or exact inverse
	if matching_order[0] >= 1:
	ome += a_s * A[0]
	if matching_order[0] >= 2:
	ome += a_s**2 * A[1]
	if matching_order[0] >= 3:
	ome += a_s**3 * A[2]
	# need inverse exact ? so add the missing pieces
	if backward_method == "exact":
	ome = np.linalg.inv(ome)

[alecandido]

I think the None option was done in other to avoid passing an additional variable is_backward to quad_ker or similar. But for sure there are other options.

indeed, now as you say I remember:

But then we are already passing to Numba something that is either expanded or "everything else".
Maybe we can simplify even backward_method by using a boolean backward_expanded: we save a string and string comparison, with something that is fully equivalent (we'd be throwing away the option of having other backward methods, but it doesn't seem to me that relevant).

I take it back, backward_method == "exact" is used down below, at most we could use integers: let's keep string comparison, for the time being.

But we can still use the two methods, and just keep the empty string at the level of Numba (Numba usage is internal, we do not need to propagate to everywhere else, and outside it, we know if it is backward or not).

[niclaurenti]

@alecandido @felixhekhorn So in the end what we decided? I revert the changes to use again strings?

[alecandido]

@alecandido @felixhekhorn So in the end what we decided? I revert the changes to use again strings?

Nono, I was talking about strings inside Numba (where for sure we can not propagate Python's enum, unless we convert to integers).

About your changes, keep them as they are. I would not really consider the option of using strings again.