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

constrained vertex fit for R(K) #89

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

ottolau
Copy link
Contributor

@ottolau ottolau commented Aug 26, 2020

Dear all,

This is a PR of putting constrained vertex fit to the common framework. I haven't checked the error matrix issue yet by now.

Best regards,
Otto

@ottolau
Copy link
Contributor Author

ottolau commented Sep 18, 2020

Dear all,

I have modularized the constrained vertex fit.

Best,
Otto

'nanoAOD_KstarMuMu_step',
'nanoAOD_KstarEE_step',
#'nanoAOD_KstarMuMu_step',
#'nanoAOD_KstarEE_step',
)
)

Choose a reason for hiding this comment

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

Hi @ottolau,
can you remove the changes to the run_nano_cfg.py ?
This is not part of your PR, will confuse the history and introduce in the master branch unwanted changes in general

@amartelli
Copy link

@ottolau by the way,
when you run the test to compare size and time,
did you run for both HEAD and PR 89 with the same processes (Kee, Kmumu, Kee, Kmumu)? or some were missing in one of the two?

thanks!

@ottolau
Copy link
Contributor Author

ottolau commented Sep 22, 2020

Hi Arabella,

Thanks! I just changed back the run_nano_cfg.py file.

For the size and timing, since the additional vertex fit is modularized, I just ran this PR and check the timing report of the new module. I ran both Kee and Kmumu, while Kmumu is negligible.

Best,
Otto

@amartelli
Copy link

thanks Otto, my question was rather if you run the same version of run_nano_cfg.py on the HEAD branch and PR branch,
asking because your PR introduced some changes in this (removing the Kee and Kmumu in particular)

@ottolau
Copy link
Contributor Author

ottolau commented Dec 7, 2020

Hi all,

Here is the validation.

Best,
Otto

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