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

Crash in wf 1325.5 caused by PR 22131 #22207

Closed
fabiocos opened this issue Feb 14, 2018 · 22 comments
Closed

Crash in wf 1325.5 caused by PR 22131 #22207

fabiocos opened this issue Feb 14, 2018 · 22 comments

Comments

@fabiocos
Copy link
Contributor

@cmsbuild
Copy link
Contributor

A new Issue was created by @fabiocos Fabio Cossutti.

@davidlange6, @Dr15Jones, @smuzaffar, @fabiocos can you please review it and eventually sign/assign? Thanks.

cms-bot commands are listed here

@fabiocos
Copy link
Contributor Author

@perrotta @slava77 @rekovic FYI

@gpetruc
Copy link
Contributor

gpetruc commented Feb 14, 2018

Uhm, apparently the MC event believes to be using prescale column 1 (read from the event), while the prescale table (retrieved from the event setup has only one column), so the thing crashes.
I can put a protection like in https://github.com/cms-sw/cmssw/blob/master/L1Trigger/L1TGlobal/src/L1TGlobalUtil.cc#L157-L164 but it would be better to understand upstream why this is happening.

@rekovic
Copy link
Contributor

rekovic commented Feb 14, 2018

attn @apana

@slava77
Copy link
Contributor

slava77 commented Feb 14, 2018

assign l1

@cmsbuild
Copy link
Contributor

New categories assigned: l1

@thomreis,@nsmith-,@rekovic you have been requested to review this Pull request/Issue and eventually sign? Thanks

@slava77
Copy link
Contributor

slava77 commented Feb 14, 2018

Notably, 1325.5 is "--era Run2_2016,run2_miniAOD_80XLegacy"
the input is a file made in 80X release.

If the column choice was different there and is a property of the 80X data, then the run2_miniAOD_80XLegacy modifier can be used to pick a more appropriate value.

@slava77
Copy link
Contributor

slava77 commented Feb 14, 2018

assign reconstruction

@cmsbuild
Copy link
Contributor

New categories assigned: reconstruction

@slava77,@perrotta you have been requested to review this Pull request/Issue and eventually sign? Thanks

@slava77
Copy link
Contributor

slava77 commented Feb 14, 2018

@gpetruc
given that we were "happy" running on 1325.5 with the older setting of ReadPrescalesFromFile, perhaps you can modify
that value for run2_miniAOD_80XLegacy and we will have the wf running

@gpetruc
Copy link
Contributor

gpetruc commented Feb 14, 2018 via email

@slava77
Copy link
Contributor

slava77 commented Feb 14, 2018

we were happy in the sense of "no crashes", but it's not clear to me if we were reading any sensible value for the L1 prescales.

right. If this lingers unanswered for more than a day, I'd rather we restore running in 1325.5 around Feb 15

@apana
Copy link
Contributor

apana commented Feb 15, 2018

Hi Slava, Giovanni,

Sorry, I was offline most of the day and only now saw this issue. I'm looking into it and will report back later today.

@fabiocos
Copy link
Contributor Author

@apana I agree with Slava, either this we have a solution by today, or better to revert the change until it is provided.

@apana
Copy link
Contributor

apana commented Feb 15, 2018 via email

@gpetruc
Copy link
Contributor

gpetruc commented Feb 15, 2018

If this is the only failing workflow, I'll then temporarily reset the parameter to false in the run2_miniAOD_80XLegacy era.

Giovanni

@gpetruc
Copy link
Contributor

gpetruc commented Feb 15, 2018

done in #22239

@fabiocos
Copy link
Contributor Author

@gpetruc @apana thanks, this is the only wf suffering a crash from the original PR by Giovanni, I think the best thing is finding a proper solution with calm. I will integrate the temporary solution by Giovanni, and when we have the fix from L1 we can just remove it.

@fabiocos
Copy link
Contributor Author

@apana I understand #22314 is meant to fix this issue, right?

@apana
Copy link
Contributor

apana commented Feb 23, 2018 via email

@fabiocos
Copy link
Contributor Author

@apana @gpetruc I have just made #22338 to go back to the original proposal by Giovanni

@fabiocos
Copy link
Contributor Author

The issue looks closed with the last PR merge.

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

6 participants