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

L1FPGATrackProducer fails in special relval with outer tracker inefficiency in CMSSW_12_6_4 #41357

Open
mmusich opened this issue Apr 17, 2023 · 14 comments

Comments

@mmusich
Copy link
Contributor

mmusich commented Apr 17, 2023

From https://its.cern.ch/jira/browse/PDMVRELVALS-148, the setup at https://cms-pdmv.cern.ch/relval/api/relvals/get_cmsdriver/CMSSW_12_6_4__RV148-TTbar_14TeV-00005 results in the following exception:

----- Begin Fatal Exception 08-Apr-2023 19:25:59 CEST-----------------------
An exception of category 'LogicError' occurred while
   [0] Processing  Event run: 1 lumi: 80 event: 15924 stream: 0
   [1] Running path 'L1TrackTrigger_step'
   [2] Calling method for module L1FPGATrackProducer/'l1tTTTracksFromExtendedTrackletEmulation'
Exception Message:
WARNING dphi and/or dphiapprox too large : -0.519923 -0.0301464
----- End Fatal Exception -------------------------------------------------

which appears to be triggered from here:

if (std::abs(dphi) > 0.5 * settings_.dphisectorHG() || std::abs(dphiapprox) > 0.5 * settings_.dphisectorHG()) {
throw cms::Exception("LogicError") << "WARNING dphi and/or dphiapprox too large : " << dphi << " " << dphiapprox
<< endl;
}

I cannot reproduce the problem by the PSet.py file at this link though I see several (probably?) related warnings coming from here:

edm::LogProblem("Tracklet") << "WARNING dphi and/or dphiapprox too large : " << dphi << " " << dphiapprox
<< "dphi " << dphi << " Seed / ISeed " << tracklet->getISeed() << endl;

@cmsbuild
Copy link
Contributor

A new Issue was created by @mmusich Marco Musich.

@Dr15Jones, @perrotta, @dpiparo, @rappoccio, @makortel, @smuzaffar can you please review it and eventually sign/assign? Thanks.

cms-bot commands are listed here

@makortel
Copy link
Contributor

assign l1, upgrade

@cmsbuild
Copy link
Contributor

New categories assigned: upgrade,l1

@epalencia,@AdrianoDee,@srimanob,@aloeliger,@cecilecaillol you have been requested to review this Pull request/Issue and eventually sign? Thanks

@BenjaminRS
Copy link
Contributor

Hi @skinnari @aryd Is it correct to have this throw an exception? The code was changed (from printout to throwing an exception) in the main branch here which comes from the cms-L1TK PR#74 here. If it is correct it would suggest we are "catching significant consistency problems in the configuration"; if so do you know what the problem is here?

@skinnari
Copy link
Contributor

hi @BenjaminRS @mmusich , @Jingyan95 tried to look at this and cannot reproduce the problem either? the change from printout to exception was done 2+ years ago... note that it is not the MatchProcessor.cc that is crashing (I'm assuming, since it's not run by default) but rather the corresponding line in MatchCalculator.cc.

@srimanob
Copy link
Contributor

Hi @Jingyan95 @skinnari

To reproduce the issue, you can use CMSSW_13_3_0_pre3 with

cmsDriver.py step2 --conditions auto:phase2_realistic_T25 --customise SLHCUpgradeSimulations/Configuration/aging.customise_aging_1000 --datatier GEN-SIM-RAW --era Phase2C17I13M9 --eventcontent RAWSIM --filein file:step1.root --fileout file:step2.root --geometry Extended2026D98 --nThreads 8 --no_exec --number -1 --python_filename step_2_cfg.py --step DIGI,L1TrackTrigger,L1,DIGI2RAW,HLT:@relval2026 --customise_commands "process.source.lumisToProcess = cms.untracked.VLuminosityBlockRange('1:80-1:80') \n process.source.eventsToProcess = cms.untracked.VEventRange('1:7970-1:7999')"

The input GEN-SIM file:
/store/relval/CMSSW_13_3_0_pre3/RelValQCD_FlatPt_15_3000HS_14/GEN-SIM/131X_mcRun4_realistic_v6_2026D98noPU-v1/2580000/5e98bf8c-aab6-44db-a434-f25ec6cd4fa0.root

You will quickly see the issue

----- Begin Fatal Exception 22-Sep-2023 12:16:40 CEST-----------------------
An exception of category 'LogicError' occurred while
   [0] Processing  Event run: 1 lumi: 80 event: 7978 stream: 2
   [1] Running path 'L1TrackTrigger_step'
   [2] Calling method for module L1FPGATrackProducer/'l1tTTTracksFromExtendedTrackletEmulation'
Exception Message:
WARNING dphi and/or dphiapprox too large : 0.57548 0.0214315
----- End Fatal Exception -------------------------------------------------

@Jingyan95
Copy link
Contributor

I was able to reproduce the problem with the Phat's recipe. The exception comes from

if (std::abs(dphi) > 0.5 * settings_.dphisectorHG() || std::abs(dphiapprox) > 0.5 * settings_.dphisectorHG()) {
throw cms::Exception("LogicError")
<< "WARNING dphi and/or dphiapprox too large : " << dphi << " " << dphiapprox << endl;
}

@Jingyan95
Copy link
Contributor

Jingyan95 commented Sep 22, 2023

Looking a bit further, the exception seems to be caused by a few events with large d0 (~25), way above the max value specified in Settings.h

I would propose two solutions:

(1) We add a cut on d0 in TrackletCalculatorDisplaced.cc similar to what's been done to d0approx already

if (std::abs(d0approx) > settings_.maxd0()) {
if (settings_.debugTracklet())
edm::LogVerbatim("Tracklet") << "Failed tracklet d0 cut " << d0approx;
success = false;
}

At the moment there is no cut on d0 (exact) in TrackletCalculatorDisplaced.cc which allows d0 go to very high and causing strange behaviour on dphi calculation

(2) If we only want to cut on d0approx not d0, we should probably remove that check on dphi

if (std::abs(dphi) > 0.5 * settings_.dphisectorHG() || std::abs(dphiapprox) > 0.5 * settings_.dphisectorHG()) {

This makes the criteria consistent across different modules.

I am not the author of these two modules so maybe somebody else can comment on this.

@srimanob
Copy link
Contributor

Hi @Jingyan95
Thanks very much for investigating it. To decide between 2 solutions, we may need to understand if it is by intention to have both d0 and d0approx or not. Who should comment on this?

@mmusich
Copy link
Contributor Author

mmusich commented Oct 9, 2023

@srimanob

Thanks very much for investigating it. To decide between 2 solutions, we may need to understand if it is by intention to have both d0 and d0approx or not. Who should comment on this?

What's the follow up item on this?

@srimanob
Copy link
Contributor

There was a discussion in L1 meeting as far as I know, and @skinnari is following up.

@mmusich
Copy link
Contributor Author

mmusich commented Oct 13, 2023

#43014 is addressing this.

@mmusich
Copy link
Contributor Author

mmusich commented Oct 17, 2023

@cms-sw/upgrade-l2 @cms-sw/l1-l2 please confirm #43014 solves the issue (e.g. by signing this issue).

@srimanob
Copy link
Contributor

We are planning to test and also produce samples. I will sign by then.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants