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

Protection to stabilize the behavior of PFClusterAlgo #2490

Merged
merged 4 commits into from Feb 22, 2014

Conversation

lgray
Copy link
Contributor

@lgray lgray commented Feb 16, 2014

Require that the sum of rechit fractions be of reasonable size to be sure you're not adding some rechit that is far away any cluster position.

An example case of the instability is as follows:
some rechit is, at closest, ~7-8 crystals away from any cluster position. This means that the largest rechit fraction is going to be ~1e-23. The smaller fractions from clusters even farther away can be ~1e-50, which is outside of double precision when you add them together. The resulting fraction can then randomly be one or very very slightly less than one.

The means that when the fractions are normalized a rechit that is very far away from any cluster can be randomly added in its entirety to a cluster it simply doesn't belong to, and it can vanish for no reason when doing the position fit in PFClusterAlgo. This makes the algorithm convergence unstable.

Requiring the rechit fraction sum to be larger than 1e-20 fixes this issue, since it ensures that at least one cluster is within about 10 shower-sigmas of the rechit. The only exception to this case are the rechit seeds, which shouldn't move very from from the cluster position on average.

Note: this can change the final positions and energies of PFClusters (especially low-energy clusters) in all sub-detectors, but should not affect higher-level physics objects.

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @lgray (Lindsey Gray) for CMSSW_7_1_X.

Protection to stabilize the behavior of PFClusterAlgo

It involves the following packages:

RecoParticleFlow/PFClusterProducer

@nclopezo, @cmsbuild, @anton-a, @thspeer, @slava77, @Degano can you please review it and eventually sign? Thanks.
You can sign-off by replying to this message having '+1' in the first line of your reply.
You can reject by replying to this message having '-1' in the first line of your reply.
@nclopezo, @ktf you are the release manager for this.
You can merge this pull request by typing 'merge' in the first line of your comment.

@cmsbuild
Copy link
Contributor

-1
When I ran the RelVals I found an error in the following worklfows:
50101.0 step2

runTheMatrix-results/50101.0_SingleMuPt10+SingleMuPt10FSIdINPUT+SingleMuPt10FS_ID/step2_SingleMuPt10+SingleMuPt10FSIdINPUT+SingleMuPt10FS_ID.log

you can see the results of the tests here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-2490/190/summary.html

@lgray
Copy link
Contributor Author

lgray commented Feb 17, 2014

This was a DAS error, retest.

@cmsbuild
Copy link
Contributor

@lgray
Copy link
Contributor Author

lgray commented Feb 18, 2014

@slava77 if it is possible, could we get this in pre3?

@lgray
Copy link
Contributor Author

lgray commented Feb 19, 2014

FYI: I looked at the automatic comparisons and they show no issues.

@anton-a
Copy link

anton-a commented Feb 20, 2014

checked the effect of the change using fwlite comparisons for single electrons (pt 35, 1000), photons (pt 10, 35), muons (pt 10, 1000), QCD (wf38.0), and ttbar (wf202.0). Some differences in the distributions for both low-level and high-level objects are observed. They are consistent with the expected small variations of energy and position of clusters and do not exhibit trends. Possible exceptions are the PS clusters where change in the spectrum is observed (a plot attached below for ttbar). Need to run some DQM comparisons before signing off as the one from jenkins are low stats.

all_pr2490_vs_orig_ttbarpuwf202p0c_log10recopfclusters_particleflowclusterps__reco_obj_energy

@lgray
Copy link
Contributor Author

lgray commented Feb 20, 2014

@anton-a I think I've identified the problem. The energy-scaled distance weight needs to be normalized to the clustering threshold. The preshower has a threshold of 6e-5, and everything else has thresholds O(1). This could cause PS hits that are actually close by to not be considered, since the 1e-20 cut is effective ~1e-15 for the PS. If we normalize this it should fix this issue. I'll push a patch in an hour or so.

@lgray
Copy link
Contributor Author

lgray commented Feb 20, 2014

@anton-a Yes, this is definitely it since the shower sigma for the PS is 0.2. 1e-20 going to 1e-15 would definitely cause the effect you see.

@lgray
Copy link
Contributor Author

lgray commented Feb 20, 2014

@anton-a here is the proposed fix

@cmsbuild
Copy link
Contributor

Pull request #2490 was updated. @nclopezo, @cmsbuild, @anton-a, @thspeer, @slava77, @Degano can you please check and sign again.

@lgray
Copy link
Contributor Author

lgray commented Feb 20, 2014

@anton-a I need to fix a minor inconsistency. Just a moment.

@lgray
Copy link
Contributor Author

lgray commented Feb 20, 2014

@anton-a Everything is good to go now. Please retest!

@cmsbuild
Copy link
Contributor

Pull request #2490 was updated. @nclopezo, @cmsbuild, @anton-a, @thspeer, @slava77, @Degano can you please check and sign again.

@cmsbuild
Copy link
Contributor

@lgray
Copy link
Contributor Author

lgray commented Feb 20, 2014

@anton-a I have added a silent protection against this case as discussed. If you give a zero threshold, it will simply normalize to one. This reproduces the original behavior of the algorithm.

@cmsbuild
Copy link
Contributor

Pull request #2490 was updated. @nclopezo, @cmsbuild, @anton-a, @thspeer, @slava77, @Degano can you please check and sign again.

@anton-a
Copy link

anton-a commented Feb 21, 2014

The changes did not fix the trend observed for the PS clusters in wf202. The new plot is virtually identical to the posted above. Will review the rest in the morning.

@lgray
Copy link
Contributor Author

lgray commented Feb 21, 2014

Ok, I'll will see if I can find out what's up on my side as well.

Question: does the preshower spectrum change in the single electron or single photon relvals?

If it does there are two clear possibilities:

Since the shower sigma is so small for the PS and it has long, thin strips (6.3 cm x 0.2 cm) stacked in either the X or Y directions, one case could be that we're now dropping energy that was being improperly clustered in because of numerical issues.

Another, more likely, scenario comes from physics-based arguments. Right now the shower sigma for the PS is 0.2. This means that one strip is one sigma away. The Moliere radius of lead is 1.6 cm, or 8 sigmas, which translates to a distance weight of 1e-14, which is safe in terms of numerics. However, the Moliere radius only defines a region of 90% containment, so there could be additional energy that actually belongs to the shower in the "unsafe" 10-11 sigma region that is cut away at present.

I'll check with some preshower experts to make sure this reasoning is sound and check in ttbar.

Update:
Based on a simple assumption I tried increasing the shower sigma to the Moliere radius for 68% containment. I include plots below that show a recovered energy spectrum, which agrees very well even into the long tails, just demonstrating some jitter. The position distributions demonstrate only jitter as well. I have updated the PR to include this adjustment to the shower sigma. The plots below are from wf202.0 (TTbar).

shower_sigma_1p0_linear
shower_sigma_1p0_log_restack
shower_sigma_1p0_log
shower_sigma_1p0_posx
shower_sigma_1p0_posy

@cmsbuild
Copy link
Contributor

Pull request #2490 was updated. @nclopezo, @cmsbuild, @anton-a, @thspeer, @slava77, @Degano can you please check and sign again.

@lgray
Copy link
Contributor Author

lgray commented Feb 21, 2014

After talking to preshower experts, the change in the energy spectrum is actually expected. The shower size in the preshower is actually very narrow! Of the order of the size of a PS strip, 0.2cm.

The numerical protection is doing its job here and restoring sane behavior, since with out the protection basically any PS hit in the topological cluster was contributing to a random PS cluster!

I will try to confirm in a bit that things are ok for real EM showers with a single photon gun.

@lgray
Copy link
Contributor Author

lgray commented Feb 21, 2014

Here are the equivalent plots for photon gun 35 GeV using the original value for shower sigma but with and without the numerical protection. High statistics will help clear up the case, but there's really not a visible change for true EM objects.

log10e_photongun35
posx_photongun35
posy_photongun35

@cmsbuild
Copy link
Contributor

Pull request #2490 was updated. @nclopezo, @cmsbuild, @anton-a, @thspeer, @slava77, @Degano can you please check and sign again.

@cmsbuild
Copy link
Contributor

@anton-a
Copy link

anton-a commented Feb 21, 2014

The effect on clusters extends mostly to PS. HCAL, and ECAL are less affected as shown on the plots for wf202
all_pr2490_mod2_vs_orig_ttbarpuwf202p0c_log10recopfclusters_particleflowclusterps__reco_obj_energy
all_pr2490_mod2_vs_orig_ttbarpuwf202p0c_log10recopfclusters_particleflowclusterhcal__reco_obj_energy
all_pr2490_mod2_vs_orig_ttbarpuwf202p0c_log10recopfclusters_particleflowclusterecal__reco_obj_energy

Higher level objects are affected, but no trends are observed and the effect is small.
all_pr2490_mod2_vs_orig_ttbarpuwf202p0c_recopfjets_ak5pfjets__reco_obj_energy

@lgray
Copy link
Contributor Author

lgray commented Feb 21, 2014

@anton-a Could you post plots for electrons as well?

@anton-a
Copy link

anton-a commented Feb 21, 2014

Single electron pt35
all_pr2490_mod2_vs_orig_singleelectron35wf17p0c_recogsfelectrons_gedgsfelectrons__reco_obj_pt
all_pr2490_mod2_vs_orig_singleelectron35wf17p0c_recogsfelectrons_gedgsfelectrons__reco_obj_energy

@anton-a
Copy link

anton-a commented Feb 22, 2014

+1
using f88a2d9 in CMSSW_7_1_X_2014-02-19-0200
and tests with wf 16, 17, 18, 19, 20, 22, 38, 202

@cmsbuild
Copy link
Contributor

This pull request is fully signed and it will be integrated in one of the next CMSSW_7_1_X IBs unless changes (tests are also fine). @nclopezo, @ktf can you please take care of it?

ktf added a commit that referenced this pull request Feb 22, 2014
Reco updates -- Protection to stabilize the behavior of PFClusterAlgo
@ktf ktf merged commit 4d93095 into cms-sw:CMSSW_7_1_X Feb 22, 2014
@nclopezo nclopezo modified the milestones: CMSSW_7_1_0_pre4, CMSSW_7_1_0_pre3 Feb 24, 2014
@nclopezo nclopezo modified the milestones: CMSSW_7_1_0_pre5, CMSSW_7_1_0_pre4 Mar 10, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants