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

Prop plane #135

Closed
wants to merge 16 commits into from
Closed

Prop plane #135

wants to merge 16 commits into from

Conversation

cerati
Copy link
Collaborator

@cerati cerati commented Oct 4, 2023

PR description:

Implement propagation to plane and kalman update to plane.
Propagation to plane is used for propagation to the hit, while propagation to r and z are used for propagation to the layer center. Propagation to the hit starts from the state at the previous hit (by re-calling the mkfndr->inputTracksAndHitIdx(...) ).
This also updates the material effects, as developed by @slava77 in pr #106.

PR validation:

http://uaf-10.t2.ucsd.edu/~cerati/prop2plane-PR/

@slava77
Copy link

slava77 commented Oct 6, 2023

This also updates the material effects, as developed by @slava77 in pr #106.

we were discussing how to interpret the results, since there are some regressions

in the initialStep
image

please clarify if the difference between red and black is only prop to plane or if there is more, like some other differences in how material is handled or something else.

@cerati
Copy link
Collaborator Author

cerati commented Oct 6, 2023

Red does not have new material nor p2plane. It just changes whether the material effects are applied before or after the error propagation.

@slava77
Copy link

slava77 commented Oct 6, 2023

Red does not have new material nor p2plane. It just changes whether the material effects are applied before or after the error propagation.

Is it possible have the new material separately to decouple the prop2plane itself. It may be just a matter of adding another reference from the earlier PR

@cerati
Copy link
Collaborator Author

cerati commented Oct 9, 2023

I set PROP_TO_PLANE = false in MkFinder.cc, which should disable all p2plane changes and keep only changes to the material. Here is the validation: http://uaf-10.t2.ucsd.edu/~cerati/prop2plane-PR-checkPR106/

@mmasciov
Copy link

mmasciov commented Oct 9, 2023

I set PROP_TO_PLANE = false in MkFinder.cc, which should disable all p2plane changes and keep only changes to the material. Here is the validation: http://uaf-10.t2.ucsd.edu/~cerati/prop2plane-PR-checkPR106/

Comparing black to orange, and orange to reference, then this would imply that the regression comes from PR #106, and that the additional changes actually reduce such a regression?

@slava77
Copy link

slava77 commented Oct 9, 2023

Comparing black to orange, and orange to reference, then this would imply that the regression comes from PR #106, and that the additional changes actually reduce such a regression?

this seems to apply to the endcap part.
The barrel part (e.g. loss of hits) seems to be from prop to plane.

@slava77
Copy link

slava77 commented Oct 12, 2023

in a follow up to our chat about free vs constrained Jacobian, it would be good to check muons, I wanted to suggest to check low pt, smth like 0.2-0.6 range so that the incidence angle in the barrel is generally relatively large

@cerati
Copy link
Collaborator Author

cerati commented Oct 12, 2023

agreed. But it would also be good to separately check larger pt muons. If there are samples this is easy enough for me to do

@slava77
Copy link

slava77 commented Oct 17, 2023

agreed. But it would also be good to separately check larger pt muons. If there are samples this is easy enough for me to do

are you working with CMSSW or with bin files? Which release?
We discussed yesterday that @leonardogiannini may help with making samples.

@cerati
Copy link
Collaborator Author

cerati commented Oct 17, 2023

Thanks. I am working with CMSSW, release CMSSW_13_3_0_pre2

@slava77
Copy link

slava77 commented Oct 18, 2023

Thanks. I am working with CMSSW, release CMSSW_13_3_0_pre2

from the earlier discussion, I guess we could benefit from some 10K events each, 10 muon events (AddAntiParticle = False to avoid continuous tracks) for

  • pt 0.1 - 1 GeV
  • pt 1 - 1000 GeV

@leonardogiannini
Copy link

Thanks. I am working with CMSSW, release CMSSW_13_3_0_pre2

from the earlier discussion, I guess we could benefit from some 10K events each, 10 muon events (AddAntiParticle = False to avoid continuous tracks) for

  • pt 0.1 - 1 GeV
  • pt 1 - 1000 GeV

despite some delay, here are the 10 muon samples for testing the tracking step:

/ceph/cms/store/user/legianni/sample-generation-CMSSW_13_3_0_pre2/step2_10muon_0p1to1_alt.root
/ceph/cms/store/user/legianni/sample-generation-CMSSW_13_3_0_pre2/step2_10muon_1to1000_alt.root

@cerati
Copy link
Collaborator Author

cerati commented Oct 27, 2023

Thanks Leonardo. Here are the results:
http://uaf-10.t2.ucsd.edu/~cerati/p2plane-MuLowPt/
http://uaf-10.t2.ucsd.edu/~cerati/p2plane-MuHiPt/
Looks like the problem at high pt is macroscopic, so hopefully I can find a good friend to spend time with:
http://uaf-10.t2.ucsd.edu/~cerati/p2plane-MuHiPt/plots_building_initialStep/hitsLayers.pdf

@cerati
Copy link
Collaborator Author

cerati commented Nov 3, 2023

The commit above fixes the high pT issue: http://uaf-10.t2.ucsd.edu/~cerati/p2plane-MuHiPt-fix/
There is still a loss of hits at low pT in the barrel. Not sure this is a problem or it just requires a new tuning of chi2 etc.

@osschar
Copy link
Collaborator

osschar commented Nov 14, 2023

Recreated as #136 ... which is in turn cms-sw#43146. Closing.

@osschar osschar closed this Nov 14, 2023
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.

5 participants