Skip to content

kfactor module maintenance #160

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

Closed
giacomomagni opened this issue Mar 15, 2024 · 4 comments · Fixed by #161
Closed

kfactor module maintenance #160

giacomomagni opened this issue Mar 15, 2024 · 4 comments · Fixed by #161
Labels
bug Something isn't working enhancement New feature or request

Comments

@giacomomagni
Copy link
Contributor

we should maintain the kfactor module, right now it isn't documented, variables names are not consistent (max_as should look somenting as order_to_update) , and there are some hardcoded pieces which most likely will not work anymore with the new commondata:

cfac_path = kfactor_folder / f"CF_QCD_{grid}.dat"
if "ATLASDY2D8TEV" in grid:
cfac_path = kfactor_folder / f"CF_QCDEWK_{grid}.dat"

@giacomomagni giacomomagni added bug Something isn't working enhancement New feature or request labels Mar 15, 2024
@alecandido
Copy link
Member

Btw: a huge rework, that could simplify it a lot, is now possible because of merge support in PineAPPL:
NNPDF/pineappl#247

This was not available at the time this module was written (same for scale variations), and because of this many files were generated and later merged.
Ok, PineAPPL subgrids are not NumPy arrays, so not all operations are available. But indexing, scalar multiplications and a combination of sum/concatenation (through merge) is. This should be enough to reimplement it as:

  • collect weights and paths (from here to there times this)
  • apply the operation in memory

@giacomomagni
Copy link
Contributor Author

Yes indeed that we can also benefit.
But also other pieces can be simplified:

max_as = to_construct_order[0] - m
alpha_val = alphas.alphasQ2(Q2)
alpha_term = 1.0 / pow(alpha_val, max_as - (nec_order[0] - m))
k_term = central_k_factor[bin_index] - 1.0
if order_exists and (max_as - (nec_order[0] - m)) == 0:
k_term = central_k_factor[bin_index]
return k_term * alpha_term

for instace here m is subtracted and added back, so can simply be dropped by the function.

@felixhekhorn felixhekhorn changed the title kfactor module mainteinece kfactor module maintenance Mar 15, 2024
@giacomomagni giacomomagni linked a pull request Mar 15, 2024 that will close this issue
3 tasks
@giacomomagni
Copy link
Contributor Author

@alecandido do you remember what happen in case of merging with an overlapping order ?
It's simply overwritten?

@alecandido
Copy link
Member

alecandido commented Mar 15, 2024

Not really, the two subgrids will get merged.

The Grid.merge() algorithm is simple: if there is only one, pick that, if there are two, call Subgrid.merge().

Merging subgrids has a different meaning for each subgrid kind, but the overall idea semantic is an addition (e.g. for the n-tuple means concatenating the tuples' lists, and for Lagrange should be actual addition of the weights).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants