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

l2kinproducer for PR to master #43

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

l2kinproducer for PR to master #43

wants to merge 1 commit into from

Conversation

NTrevisani
Copy link

No description provided.

@NTrevisani
Copy link
Author

Please @BlancoFS, @mlizzo, and @dittmer have a look. I prepared this module, so I cannot review it :)

@BlancoFS
Copy link

BlancoFS commented Oct 9, 2024

Hi @NTrevisani,

In general, I would say it looks ready to merge. I don't see any missing variable with the exception of the mT2. Is this variable intended to be included? Could we just copy this function within the ROOT interpreter and run it on RDF?

Then, maybe this is not the right place to comment but I remember there was an implementation of the l3kin variables. Do we want to include it in the PR? (Or probably in a different PR)

@NTrevisani
Copy link
Author

Hi @BlancoFS,

Thank you for the suggestions.

About mt2, I think we decided to avoid including it since it is not used anymore and it may add some time to compute it. Please @mlizzo and @dittmer comment on this if you have a different opinion.

I agree we should include also the l3kin and l4kin modules. I have to check if they are ready or not.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants