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

Integration to CMSSW master #236

Closed
17 of 18 tasks
makortel opened this issue Aug 13, 2019 · 15 comments
Closed
17 of 18 tasks

Integration to CMSSW master #236

makortel opened this issue Aug 13, 2019 · 15 comments

Comments

@makortel
Copy link
Collaborator

makortel commented Aug 13, 2019

This issue is to track progress towards integrating mkFit to CMSSW master:

Overall (to get mkFit running within offline reco)

Getting mkFit to run within HLT reconstruction some additional changes may be wanted

@slava77
Copy link
Collaborator

slava77 commented Aug 13, 2019

either re-open and merge cms-sw/cmssw#26322, or remove or allow chancing the minimum hit cut for seeds in the seed cleaning

I'm not sure that the CMSSW PR reopening will go so easily, because the original issue there is not addressed. Pattern reco turns out to deal with the outer hits better compared to the seeding. It didn't look like the POG was willing to commit to making things slightly worse under an assumption that it will all be done better anyways for the final Run 3 tracking.
Perhaps that regression goes away with the patatrack seeds with refit, but this will apparently not show up in CMSSW for some time.

In the current iterative tracking that we are using for tests all seeds are of the same length. So, this selection doesn't really make sense on a seed by seed level.
I propose to drop this cut.

@slava77
Copy link
Collaborator

slava77 commented Aug 13, 2019

either re-open and merge cms-sw/cmssw#26322,

Oh, if you can percolate this addition as configurable, there should be no problem to get this PR in.

@makortel
Copy link
Collaborator Author

@slava77 tagged a new release https://github.com/trackreco/mkFit/releases/tag/V2.0.0-1+pr237

I have the full chain tested locally up to a runTheMatrix workflow running with a customize function changing the usual initialStep pattern recognition with mkFit.

@makortel
Copy link
Collaborator Author

The cmsdist PR is here cms-sw/cmsdist#5150

@makortel
Copy link
Collaborator Author

Apparently cms-sw/cmsdist#5150 (merged on last Friday) was not sufficient to get the mkfit external usable from CMSSW, so I made a new PR to add the (hopefully last) missing piece cms-sw/cmsdist#5169.

@slava77
Copy link
Collaborator

slava77 commented Aug 28, 2019

we should also add some "native" support to replace patches prepared in cms-sw/cmsdist#5174
@osschar you are probably the most familiar with the intrinsics to get it right.

@makortel
Copy link
Collaborator Author

we should also add some "native" support to replace patches prepared in cms-sw/cmsdist#5174
@osschar you are probably the most familiar with the intrinsics to get it right.

Let's follow up those in #240.

@osschar
Copy link
Collaborator

osschar commented Aug 28, 2019

Well, it's not just what's the subject of #240 is (headers) ... we also need to:

  1. massage the makefile, have some architecture detection in there
  2. figure out how much time we want to spend on getting Matriplex intrinsic vectorization to work on arm and ppc (my guess is zero at this point) ... so the easiest would be to simply fall back to GenMul generating standard c++ and let the compilers try to vectorize that.
    In any case, we need access to those machines with compilers cms uses to try things out.

@makortel
Copy link
Collaborator Author

The CMSSW PR is here cms-sw/cmssw#27895

@makortel
Copy link
Collaborator Author

makortel commented Sep 6, 2019

Just to document here that in light of the presentation at TRK POG on Monday showing the performance with #241, we just decided to cut a new release after its merge (now), and update the version in cmsdist.

@makortel
Copy link
Collaborator Author

makortel commented Sep 6, 2019

Just to document here that in light of the presentation at TRK POG on Monday showing the performance with #241, we just decided to cut a new release after its merge (now), and update the version in cmsdist.

We have a new release https://github.com/trackreco/mkFit/releases/tag/V2.0.1-0+pr241

@makortel
Copy link
Collaborator Author

makortel commented Sep 6, 2019

Just to document here that in light of the presentation at TRK POG on Monday showing the performance with #241, we just decided to cut a new release after its merge (now), and update the version in cmsdist.

We have a new release https://github.com/trackreco/mkFit/releases/tag/V2.0.1-0+pr241

cmsdist PR is here cms-sw/cmsdist#5213

@makortel
Copy link
Collaborator Author

I'm leaving the issue open though until the HLT part is resolved as well.

@makortel
Copy link
Collaborator Author

makortel commented Apr 9, 2021

This issue is effectively resolved

@makortel makortel closed this as completed Apr 9, 2021
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

No branches or pull requests

3 participants