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

Pr80x l1t fix crash XMLConfigReader with localize xcerces #16198

Conversation

rekovic
Copy link
Contributor

@rekovic rekovic commented Oct 13, 2016

This is 80x version of 81x PR #16194 by @davidlange6,

It fixes the problem in end of job crash in workflow 136.7321.

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @rekovic for CMSSW_8_0_X.

It involves the following packages:

L1Trigger/L1TCommon
L1Trigger/L1TMuonBarrel
L1Trigger/L1TMuonOverlap

@cmsbuild, @rekovic, @mulhearn, @davidlange6 can you please review it and eventually sign? Thanks.
@Martin-Grunewald this is something you requested to watch as well.
@slava77, @smuzaffar you are the release manager for this.

cms-bot commands are listed here #13028

@rekovic
Copy link
Contributor Author

rekovic commented Oct 13, 2016

@kkotov As suggested #16194 (comment), please use this PR to test O2O functionality after these changes.

@rekovic
Copy link
Contributor Author

rekovic commented Oct 13, 2016

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Oct 13, 2016

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/15705/console

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

@kkotov
Copy link
Contributor

kkotov commented Oct 13, 2016

@rekovic , @mulhearn , I don't think I fully understand the history of changes. First of all, I see that the input xmls merged to CMSSW in some earlier PRs differ wrt. what I always had in the O2O:
https://github.com/cms-sw/cmssw/blob/CMSSW_8_0_X/L1Trigger/L1TMuonOverlap/python/fakeOmtfParams_cff.py
and
https://github.com/kkotov/cmssw/blob/rebasedFullL1TO2Ofor80X/L1Trigger/L1TMuonOverlap/python/fakeOmtfParams_cff.py

If I disregard this difference then, irrelevant of the O2O, the ESProducer generates identical payload before and after the fixes of this PR. That can be viewed using a following recipe in the latest IB with and without this PR:
cp /afs/cern.ch/user/k/kkotov/public/viewOverPar.py ./
cmsRun ./viewOverPar.py

Finally, the O2O at p5 uses CMSSW_8_0_18 which turned out to be out-of-date with the recent OMTF config code in the IB, e.g.:
diff /cvmfs/cms.cern.ch/slc6_amd64_gcc493/cms/cmssw/CMSSW_8_0_18/src/L1Trigger/L1TMuonOverlap/src/XMLConfigReader.cc /cvmfs/cms-ib.cern.ch/2016-42/slc6_amd64_gcc530/cms/cmssw/CMSSW_8_0_X_2016-10-09-0000/src/L1Trigger/L1TMuonOverlap/src/XMLConfigReader.cc
I am not sure what part of the code affects the observed O2O behaviour, but I see that the chargeLUT, etaLUT, ptLUT, pdfLUT, and meanDistPhiLUT are now different (even if I force the fakeOmtfParams_cff.py to read the same xmls). Interestingly, the other 6 LUTs are still identical.

@rekovic
Copy link
Contributor Author

rekovic commented Oct 13, 2016

@kkotov My first quick comment: the difference in fakeOmtfParams_cff.py is understood.
Your O2O branch contained the old version with b2556d1, while the most recent version was provided in l1t-integration with 9f6dafe.

@kkotov
Copy link
Contributor

kkotov commented Oct 13, 2016

@rekovic my understanding from OMTF was that they change their configuration very infrequently. Every time they change it, they let us know about it and acknowledge the change in using a new version of the online key. Since I never heard about any changes this year and because the key stayed the same, I did not expect the difference that I saw. Let's follow it up with Artur in the next emulators meeting.

@davidlange6
Copy link
Contributor

Hi @rekovic - are we ready to proceed with this? It looks like the 81x IBs are running ok (I need to check whats happened on the memory side of things)

@rekovic
Copy link
Contributor Author

rekovic commented Oct 16, 2016

+1

@davidlange6
The change to code made here leave the O2O functionality unchanged, as confirmed by @kkotov.
Now, I see the branch has conflicts in something underneath us: Alignment/MillePedeAlignmentAlgorithm/templates/alignment_config.ini
Otherwise OK.

@cmsbuild
Copy link
Contributor

This pull request is fully signed and it will be integrated in one of the next CMSSW_8_0_X IBs (tests are also fine). This pull request requires discussion in the ORP meeting before it's merged. @slava77, @davidlange6, @smuzaffar

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

4 participants