cms-L1TK / cmssw

Fork of CMSSW where improvements to L1 tracking code are developed.
http://cms-sw.github.io/
Apache License 2.0
4 stars 5 forks source link

New KF (hand merged with L1TK-dev-12_0_0_pre4) #88

Closed tschuh closed 3 years ago

tschuh commented 3 years ago

replaces https://github.com/cms-L1TK/cmssw/pull/51 This PR contains my current code base and allows to run the KF after the Tracklet Pattern Reconstruction.

If you want to run the code check out

L1Trigger/TrackerDTC
L1Trigger/TrackerTFP
L1Trigger/TrackFindingTracklet
L1Trigger/TrackTrigger
DataFormats/L1TrackTrigger
SimTracker/TrackTriggerAssociation

from branch tschuh12 and run after compiling L1Trigger/TrackFindingTracklet/test/HybridTracksNewKF_cfg.py Events=10

Things that changed in DTC code:

tomalin commented 3 years ago

If the comments from #51 describing this PR are still accurate please add them to the comment at the top of this PR.

tomalin commented 3 years ago

I've verified (by eye) that no other code has been lost due to git errors.

tomalin commented 3 years ago

Please do: "git cms-checkout-topic -u cms-L1TK:L1TK-dev-12_0_0_pre4; git cms-rebase-topic -u cms-L1TK:tschuh12" to update your branch with two small PRs made by other people.

tomalin commented 3 years ago

The job L1Trigger/TrackFinderTracklet/test/L1TrackNtupleMaker_cfg.py no longer runs, as it includes the line (L111) " process.load('L1Trigger.TrackerDTC.ProducerES_cff')", which attempts to access a non-existant file. I think this line can just be deleted.

tomalin commented 3 years ago

1) L1TrackNtupleMaker_cfg.py run on 1k ttbar+200PU events says efficiency = 94.8%. 2) HybridTracksNewKF_cfg.py on same MC says in end-of-job summary that efficiency = 95.0% after KFin, but this drops to 90.0% after KF and to 0% after KFout. 3) HybridTracksNewKF_cfg.py rerun with "UseTTStubAssMap = False" says in end-of-job summary that efficiency = 94.8% after KFin, dropping to 94.6% after KF and to 0% after KFout.

Is the poor performance of (2) understood? And why do both (2) & (3) have zero efficiency after KFout? Both (2) & (3) also report zero tracks found after KFout.

tschuh commented 3 years ago

Could you run L1TrackNtupleMaker_cfg.py with new KF so that we have a full picture? I never tested UseTTStubAssMap = True setting extensively, might be buggy. Efficiency after KFout is 0 because KFout is missing.

tomalin commented 3 years ago

If KFout is not run in Hybrid, then I recommend that the printout for it should be suppressed in the Hybrid end-of-job summary. Though if it is not run, how can the KFout FW module be validated?

tomalin commented 3 years ago

Please add a PoolOutputModule to HybridTracksNewKF_cfg.py, so that the TTTracks it makes can be studied on run through the L1TrackNtupleMaker. Either comment out the PoolOutputModule (so not run by default), or use a python boolean as in L1TrackNtupleMaker_cfg.py to enable/disable it.

tschuh commented 3 years ago

KFout FW module is not validated.

tomalin commented 3 years ago

In HybridTracksNewKF_cfg.py, the TrackTriggerAssociator_cff should be run on the output TTTrack collection. (Though there seem to be lots of these, one from each algo step, so perhaps just run it on final one). Similar thing already done in HybridTracks_cfg.py

tomalin commented 3 years ago

After your bug fix, end-of-job summary results from HybridTracksNewKF_cfg.py on 1k ttbar+200PU are now: 1) efficiency = 96.15% after KFin (better), but still 90.0% after KF (bad). 2) Setting "UseTTStubAssMap = False" gives efficiency = 95.9% after KFin (better), dropping to 94.9% after KF (better).

tomalin commented 3 years ago

As the DTC code has changed, should all L1 tracking algos running on 11.3 MC rerun the DTC code?

tschuh commented 3 years ago

that sounds sensible

tomalin commented 3 years ago

I've modified L1TrackNtupleMaker_cfg.py to add the option to run Hybrid with the new KF. You can find the new code on the RAL linux computer in ~tomalin/L1TrackNtupleMaker_cfg.py . The extra code is quite long, so consider if a cff.py file could be added to define an alias to make it shorter.

tomalin commented 3 years ago

If I run L1Trigger/TrackFindingTMTT/test/test_cfg.py on 100 ttbar+200PU events, the end-of-job summary says the "max tracking efficiency" is 96% in the GP summary, but only 35% in the HT summary.

tschuh commented 3 years ago

I guess you mean L1Trigger/TrackerTFP/test/test_cfg.py. There are two reasons for the efficiency loss: 1) running 3 GeV tracking but measuring 2 GeV tracking Efficiency and 2) using hybrid data formats and running tmtt.

tomalin commented 3 years ago

I guess you mean L1Trigger/TrackerTFP/test/test_cfg.py. There are two reasons for the efficiency loss: 1) running 3 GeV tracking but measuring 2 GeV tracking Efficiency and 2) using hybrid data formats and running tmtt.

Adding to test_cfg.py the lines:

Set cfg parameters to those required by TMTT tracking

process.TrackTriggerDataFormats.UseHybrid = False process.TrackTriggerSetup.TrackingParticle.MinPt = 3.0 process.TrackTriggerSetup.Firmware.MaxdPhi = 0.01

increases the HT efficiency to 97.4% and KFin efficiency to 95.2%. But the KF efficiency is only 91.8%.

tomalin commented 3 years ago

The job HybridTracks_cfg.py in L1Trigger/TrackFindingTracklet/test/ crashes, because https://github.com/cms-L1TK/cmssw/blob/tschuh12/L1Trigger/TrackFindingTracklet/plugins/L1FPGATrackProducer.cc#L249 searches for the python configuration parameter FakeFit, but this is not defined in module TTTracksFromTrackletEmulation of https://github.com/cms-L1TK/cmssw/blob/tschuh12/L1Trigger/TrackFindingTracklet/python/Tracklet_cfi.py .

tschuh commented 3 years ago

what you should see in the report is that some percent are lost due to truncation, that is because we moved from seed filter to a hough transform in the rz plane without tuning the kf worker.

tomalin commented 3 years ago

what you should see in the report is that some percent are lost due to truncation, that is because we moved from seed filter to a hough transform in the rz plane without tuning the kf worker.

KF reports: tracking efficiency = 0.9182 +- 0.0071 lost tracking efficiency = 0.0181 +- 0.0035

Is the "lost tracking efficiency" due to trucation? (If so, I suggest replacing "lost tracking efficiency = " by "where truncation loss = ". (Otherwise, one doesn't know if the loss is due to truncation or an imperfect algorithm).

tschuh commented 3 years ago

you requested in the past that I shall add "'Lost' below refers to truncation losses" in top of the output. Shall I now remove that line and replace "lost tracking efficiency" with "where truncation loss"?

tomalin commented 3 years ago

you requested in the past that I shall add "'Lost' below refers to truncation losses" in top of the output. Shall I now remove that line and replace "lost tracking efficiency" with "where truncation loss"?

I'd missed that. OK, good as it is.

tomalin commented 3 years ago

Please check that the key comments made during https://github.com/cms-L1TK/cmssw/pull/51 have been addressed. e.g. This requested a README.md file to help people get familiar with the code, but L1Trigger/TrackerTFP/README.md contains only "tbd".

tschuh commented 3 years ago

Are there somewhere READMEs for Trigger/TrackFindingTracklet or Trigger/TrackFindingTMTT which I could use as template?

tomalin commented 3 years ago

Are there somewhere READMEs for Trigger/TrackFindingTracklet or Trigger/TrackFindingTMTT which I could use as template?

I wouldn't advise using TrackFindingTMTT/README.md as a template. But your code is unusually complex, so your README.md should give people an indication of what its key ingredients are and where they can be found. https://github.com/cms-L1TK/cmssw/pull/51#issuecomment-796792593 mentions some of these.

There are also a few details, that may be worth explaining. e.g. https://github.com/cms-L1TK/cmssw/blob/tschuh12/L1Trigger/TrackTrigger/python/ProducerSetup_cfi.py#L11 specifies the tracker geometry. When & how do these cfg params need updating as CMS releases new Tracker geometries?

tomalin commented 3 years ago

Update on tracking performance with L1TrackNtupleMaker_cfg.py on 1k ttbar+200PU, with performance obtained from L1TrackNtuplePlot.cc

1) TMTT (old emulation): efficiency = 96.0%; no. reco tracks = 223 2) Tracklet: efficiency = 95.5%; no. reco tracks = 175 3) Hybrid (old KF): efficiency = 94.8%; no. reco tracks = 166 4) Hybrid (new KF): efficiency = 95.5%; no. reco tracks = 165

This all seems good. And the New KF is performing better than the old one. However, if I look at the end-of-job summary of KF of (4), it says:

4a) Hybrid (new KF): efficiency = 94.9%, no. reco tracks per TFP = 21.4, so 21.4*9 = 192.6 in total

The reason that (4a) reports more tracks (4) is that L1TrackNtuplePlot.c doesn't count tracks with Pt < 2 GeV or eta > 2.4. (I'll investigate how to fix this).

The reason (4a) reports lower efficiency than (4) is that AnalyzerKF.cc doesn't allow up to 1 incorrect 2S stub on the track. If I remake the result (4) after configuring the association to allow 0 incorrect 2S stubs, its reported efficiency drops to 94.8%.

tomalin commented 3 years ago

On related topic: the KF end-of-job summary will go wrong if the New KF is modified in future to allow > 4 stubs per track. A suggestion to fix this https://github.com/cms-L1TK/cmssw/pull/51#discussion_r617422662 has not been addressed.

tschuh commented 3 years ago

Could you please read my README and give me feedback?

tomalin commented 3 years ago

Could we make end-of-job printout configurable, so it could quote efficiencies either allowing 1 incorrect 2S stub (to give official numbers) or with 0 incorrect 2S stub (which I understand is helpful for debugging)?

tschuh commented 3 years ago

I think that is not needed. It should now report the official number at the end and when I do a debug session I can change that to what I need for that time period.

tomalin commented 3 years ago

Could you please read my README and give me feedback?

The README looks a good start. I committed to it a few English corrections. I also added a few "CHECK" statements. Please check these and fix them.

tomalin commented 3 years ago

With the latest code, the end-of-job number (4a) doesn't change, so still disagrees with the number (4) from the L1TrackNtupleMaker.cc. The reason for this is that (4a) requires the L1 track to share stubs in at least 4 layers with the TP, in order for it to be considered as reconstructed, and so count in the numerator of the tracking efficiency. This differs from (4), which will accept the TP in the numerator if it shares stubs in only 3 layers with the L1 track, and the L1 track had one additional fake 2S stub. I suspect your definition of efficiency makes more sense. I'll arrange a discussion of this.

tomalin commented 3 years ago

After above fixes, L1Trigger/TrackFindingTMTT/test/test_cfg.py on 100 ttbar+200PU events gives for TMTT (New KF) algo end-of-job summary says KF has 91.1% efficiency (2.2% truncation), KFin has 94.5% efficiency (no truncation), ZHT has 94.8% efficiency (no truncation), MHT has 95.8% efficiency (no truncation), HT has 97.2% efficiency (no truncation), GP has 99.4% efficiency. Presumably due to not being optimised.

tomalin commented 3 years ago

Please indicate which of the comments above you have addressed.

tomalin commented 3 years ago

This comment about the tracker geometry https://github.com/cms-L1TK/cmssw/pull/51#discussion_r617854817 should be addressed. -- As the SupportedGeometry cfg paramin TrackTrigger/python/ProducerSetup_cfi.py refers to one specific geom, I'm concerned that the code may break when a new tracker geometry is released. We need a recipe to keep "SupportedGeometry" up to date.

Similarly, if the geometry changes, so that the geometrical constants in L1Trigger/TrackTrigger/python/ProducerSetup_cfi.py , such as Disk2SRsSet are incorrect, what will happen? Will the code go completely wrong, or just very slightly wrong? And will there be any warning messages? Where do these numbers come from?