Quieting 76x step2 and step3 #11963
Quieting 76x step2 and step3 #11963
Conversation
|
A new Pull Request was created by @davidlange6 (David Lange) for CMSSW_7_6_X. Quieting 76x step2 and step3 It involves the following packages: CommonTools/RecoAlgos @perrotta, @smuzaffar, @civanch, @Dr15Jones, @cvuosalo, @boudoul, @fwyzard, @franzoni, @mdhildreth, @fabozzi, @Martin-Grunewald, @srimanob, @cmsbuild, @deguio, @slava77, @vadler, @vanbesien, @monttj, @danduggan can you please review it and eventually sign? Thanks. |
|
cmsbuild, please test |
|
-1 I disagree with this change for HLT. Quieting the printout sweeps this under the rug and BTW, I also wonder why you do NOT accept our other PR which get rid of some LogErrors. |
if you have a counter proposal let me know - otherwise, you need to propose how to buy PB of tape for log files
which one?
|
|
Hi - My comment refers to the following: a) HLTrigger/HLTcore - I assume you want to silence this message: b) Scouting PR #11632 - and YES, TSG does care about 75X! |
|
I agree with Martin: the `LogError` in
`HLTrigger/HLTcore/src/HLTPrescaleProvider.cc`
are there to point out actual problems in the input data or in the job
configuration.
.A
|
|
yes, its known for a few months now…seems it should have impacted our release validation.
|
|
On 19 October 2015 at 17:36, David Lange [email protected] wrote:
It is really not my problem if people do not look at the output of the PAT It is my concern if you make all uses of a generic HLT tool fail silently .Andrea |
|
| @@ -86,7 +86,7 @@ VersionedIdProducer(const edm::ParameterSet& iConfig) { | |||
| } | |||
|
|
|||
| if( idMD5 != calculated_md5 ) { | |||
| edm::LogError("IdConfigurationNotValidated") | |||
| edm::LogInfo("IdConfigurationNotValidated") | |||
There was a problem hiding this comment.
@lgray
please note
If I understand correctly, with md5 check machinery not working, a noticeable feature of VIDs to be able to validate the configuration is dead.
There was a problem hiding this comment.
Still trying to understand it. I am somewhat annoyed if I have to keep updating it over time for no reason...
Also, other bigger things in front of it. It's OK to turn it off for now.
I'll hopefully be able to turn it back on later.
There was a problem hiding this comment.
Also, wouldn't say that a major feature is missing. It's possible to check git hashes as well (which appear to be more stable somehow...), so there are ways around it and it's not "a significantly missing piece and the validation is dead otherwise". One that is rather nice to have though, still possible to validate by git repo states.
so where does one expect the actual bug to be? we have two unproductive HN discussions already in hopes of having an expert help digest the inconsistency. PatTriggerProducer does not seem to be doing something fancy
|
| @@ -41,7 +41,7 @@ PixelVertexProducer::PixelVertexProducer(const edm::ParameterSet& conf) | |||
| track_pt_min = PVcomparerPSet.getParameter<double>("track_pt_min"); | |||
| if (track_pt_min != ptMin_) { | |||
| if (track_pt_min < ptMin_) | |||
| edm::LogWarning("PixelVertexProducer") << "minimum track pT setting differs between PixelVertexProducer (" << ptMin_ << ") and PVcomparer (" << track_pt_min << ") [PVcomparer considers tracks w/ lower threshold than PixelVertexProducer does] !!!"; | |||
| edm::LogInfo("PixelVertexProducer") << "minimum track pT setting differs between PixelVertexProducer (" << ptMin_ << ") and PVcomparer (" << track_pt_min << ") [PVcomparer considers tracks w/ lower threshold than PixelVertexProducer does] !!!"; | |||
There was a problem hiding this comment.
I guess @silviodonato can comment... if this is firing we have to fix the config I guess... can you clarify in which context was it firing too often?
There was a problem hiding this comment.
you can check, e.g. 134.702 in step2
There was a problem hiding this comment.
I think it happens once per job
There was a problem hiding this comment.
@silviodonato can you have a look?
There was a problem hiding this comment.
@davidlange6 are we going to make 5 billion jobs for 5 billion events? that's inefficient :-) This Warning is at construction time right? so it complains once per job at most... I guess config warning should be print once per job ... we have no way to print "once per production campaign" given that jobs do not talk with each other
There was a problem hiding this comment.
sure - but why keep warnings around for years? Its not “new"
On Oct 21, 2015, at 3:58 PM, arizzi [email protected] wrote:
In RecoPixelVertexing/PixelVertexFinding/src/PixelVertexProducer.cc:
@@ -41,7 +41,7 @@ PixelVertexProducer::PixelVertexProducer(const edm::ParameterSet& conf)
track_pt_min = PVcomparerPSet.getParameter("track_pt_min");
if (track_pt_min != ptMin_) {
if (track_pt_min < ptMin_)
- edm::LogWarning("PixelVertexProducer") << "minimum track pT setting differs between PixelVertexProducer (" << ptMin_ << ") and PVcomparer (" << track_pt_min << ") [PVcomparer considers tracks w/ lower threshold than PixelVertexProducer does] !!!";
- edm::LogInfo("PixelVertexProducer") << "minimum track pT setting differs between PixelVertexProducer (" << ptMin_ << ") and PVcomparer (" << track_pt_min << ") [PVcomparer considers tracks w/ lower threshold than PixelVertexProducer does] !!!";
@davidlange6 are we going to make 5 billion jobs for 5 billion events? that's inefficient :-) This Warning is at construction time right? so it complains once per job at most... I guess config warning should be print once per job ... we have no way to print "once per production campaign" given that jobs do not talk with each other
—
Reply to this email directly or view it on GitHub.
There was a problem hiding this comment.
Andrea
I’m silencing in the message logger using the same-since-ever configuration for production. The issue that no one seems to be aware of this message present during all 2015 production is a sign of how useful it is. Certainly there is plenty of time to understand the consequence now that I’ve raised awareness of it for the 2016 menus. We will continue to have it for 74x and 75x data taking in case further reminders are needed.
On Oct 21, 2015, at 3:51 PM, Andrea Bocci [email protected] wrote:
In RecoPixelVertexing/PixelVertexFinding/src/PixelVertexProducer.cc:
@@ -41,7 +41,7 @@ PixelVertexProducer::PixelVertexProducer(const edm::ParameterSet& conf)
track_pt_min = PVcomparerPSet.getParameter("track_pt_min");
if (track_pt_min != ptMin_) {
if (track_pt_min < ptMin_)
- edm::LogWarning("PixelVertexProducer") << "minimum track pT setting differs between PixelVertexProducer (" << ptMin_ << ") and PVcomparer (" << track_pt_min << ") [PVcomparer considers tracks w/ lower threshold than PixelVertexProducer does] !!!";
- edm::LogInfo("PixelVertexProducer") << "minimum track pT setting differs between PixelVertexProducer (" << ptMin_ << ") and PVcomparer (" << track_pt_min << ") [PVcomparer considers tracks w/ lower threshold than PixelVertexProducer does] !!!";
On 21 October 2015 at 15:41, David Lange [email protected] wrote: we really need more than 5 billion MC events worth of the printout?
As I did write: no. You can silence it in the MessageLogger configuration. I don't think it makes sense to make the change in the HLT menu now - the next occasion to review the impact of the change (though none is expected) would be next Thursday, with only few days of data taking left. In any case, do not downgrade the LogWarning to a LogInfo, just because it is doing its job of catching the problem it was intended to catch. .Andrea
—
Reply to this email directly or view it on GitHub.
There was a problem hiding this comment.
because nobody complained/noticed it earlier, and the fix is "fix the config".
It is not "years" it is at most "1 year", in fact less given the the config change was integrated (checking JIRA) on Nov 5th 2014
There was a problem hiding this comment.
it seems we now agree the printout has accomplished its goal. if things return back to the state where it indicates a real unknown problem, it would then be good to reenable by default as a warning. Meanwhile we can more easily see other warnings
On Oct 21, 2015, at 4:10 PM, arizzi [email protected] wrote:
In RecoPixelVertexing/PixelVertexFinding/src/PixelVertexProducer.cc:
@@ -41,7 +41,7 @@ PixelVertexProducer::PixelVertexProducer(const edm::ParameterSet& conf)
track_pt_min = PVcomparerPSet.getParameter("track_pt_min");
if (track_pt_min != ptMin_) {
if (track_pt_min < ptMin_)
- edm::LogWarning("PixelVertexProducer") << "minimum track pT setting differs between PixelVertexProducer (" << ptMin_ << ") and PVcomparer (" << track_pt_min << ") [PVcomparer considers tracks w/ lower threshold than PixelVertexProducer does] !!!";
- edm::LogInfo("PixelVertexProducer") << "minimum track pT setting differs between PixelVertexProducer (" << ptMin_ << ") and PVcomparer (" << track_pt_min << ") [PVcomparer considers tracks w/ lower threshold than PixelVertexProducer does] !!!";
because nobody complained/noticed it earlier, and the fix is "fix the config".
It is not "years" it is at most "1 year", in fact less given the the config change was integrated (checking JIRA) on Nov 5th 2014—
Reply to this email directly or view it on GitHub.
|
On 10/19/15 10:47 AM, Andrea Bocci wrote:
A less invasive alternative would be
|
|
many jenkins workflows show changes like this I think this is all due to removal of RAW2DIGI,L1Reco, but experts better check |
|
regarding #11963 (comment) |
|
+1
|
| @@ -328,7 +328,7 @@ EmDQM::dqmBeginRun(edm::Run const &iRun, edm::EventSetup const &iSetup) | |||
| edm::LogPrint("EmDQM") << "No number of candidates for filter " << moduleLabel << " found. Set to " << paramSet.getParameter<int>("cutnum") << ", determined from path name."; | |||
| filterPSet.addParameter<int>("ncandcut", paramSet.getParameter<int>("cutnum")); | |||
| } else if (filterPSet.getParameter<int>("ncandcut") > paramSet.getParameter<int>("cutnum")) { | |||
| edm::LogPrint("EmDQM") << "Changed required number of candidates from " << paramSet.getParameter<int>("cutnum") << " to " << filterPSet.getParameter<int>("ncandcut") << " for filter " << moduleLabel; | |||
| edm::LogInfo("EmDQM") << "Changed required number of candidates from " << paramSet.getParameter<int>("cutnum") << " to " << filterPSet.getParameter<int>("ncandcut") << " for filter " << moduleLabel; | |||
There was a problem hiding this comment.
Again, rather than changing the LogPrint to a LogInfo, why not change it to a LogWarning and fix the configuration ?
There was a problem hiding this comment.
Sounds great. as I said in the PR itself "if these are considered bugs, we can reenable them together with the bug fix..” Maybe @deguio wants to comment on the interest of the developers to fix
On Oct 22, 2015, at 9:37 AM, Andrea Bocci [email protected] wrote:
In HLTriggerOffline/Egamma/src/EmDQM.cc:
@@ -328,7 +328,7 @@ EmDQM::dqmBeginRun(edm::Run const &iRun, edm::EventSetup const &iSetup)
edm::LogPrint("EmDQM") << "No number of candidates for filter " << moduleLabel << " found. Set to " << paramSet.getParameter("cutnum") << ", determined from path name.";
filterPSet.addParameter("ncandcut", paramSet.getParameter("cutnum"));
} else if (filterPSet.getParameter("ncandcut") > paramSet.getParameter("cutnum")) {
edm::LogPrint("EmDQM") << "Changed required number of candidates from " << paramSet.getParameter<int>("cutnum") << " to " << filterPSet.getParameter<int>("ncandcut") << " for filter " << moduleLabel;edm::LogInfo("EmDQM") << "Changed required number of candidates from " << paramSet.getParameter<int>("cutnum") << " to " << filterPSet.getParameter<int>("ncandcut") << " for filter " << moduleLabel;Again, rather than changing the LogPrint to a LogInfo, why not change it to a LogWarning and fix the configuration ?
—
Reply to this email directly or view it on GitHub.
|
+1 |
|
@davidlange6, I discovered a flawed logic in Note that what triggered the printouts in the first place were changes made in #10858 which seem to break the PAT jet flavor workflow. So this very example shows the importance of keeping LogWarnings. With LogInfo the problem introduced in #10858 would not be caught. |
|
@davidlange6, the parameter value change introduced in #10858 that was triggering the LogWarning printouts is now fixed in #12080. |
Quieting 76x step2 and step3
|
ping , let bot update the signatures here ( new signatures due to packages assigned to new categories) |
|
Pull request #11963 was updated. @vlimant, @smuzaffar, @tjavaid, @srimanob, @simonepigazzini, @AdrianoDee, @sunilUIET, @antoniovagnerini, @tvami, @aloeliger, @epalencia, @rvenditti, @syuvivida, @nothingface0, @Dr15Jones, @miquork, @makortel can you please check and sign again. |
|
+1
|
|
@tvami , we are just testing the new bot feature on old PRs to see how is behaves for those. So no need to sign/resign already merged PR :-) |
Remove lots of printouts from 76x. Specifically these.. if these are considered bugs, we can reenable them together with the bug fix..
%MSG-w PixelVertexProducer: PixelVertexProducer:hltFastPVPixelVertices@ctor 19-Oct-2015 14:01:38 CEST pre-events
minimum track pT setting differs between PixelVertexProducer (1) and PVcomparer (0.1) [PVcomparer considers tracks w/ lower threshold than PixelVertexProducer does] !!!
%MSG
Data is now served from cern.ch instead of previous
Added 'egmGsfElectronIDs' to process definition (MiniAOD format)!
Added ID 'cutBasedElectronID-PHYS14-PU20bx25-V2-standalone-loose' to egmGsfElectronIDs
Watch modules:
Ignore modules: PhotonConversionTrajectorySeedProducerFromSingleLeg:photonConvTrajSeedFromSingleLeg, SeedGeneratorFromRegionHitsEDProducer:regionalCosmicTrackerSeeds,
Ignore categories:
Watch categories: TooManyClusters,
MSG-e IdConfigurationNotValidated: VersionedGsfElectronIdProducer:egmGsfElectronIDs@ctor 19-Oct-2015 14:25:34 CEST pre-events
ID: cutBasedElectronID-Spring15-25ns-V1-standalone-loose
The expected md5: 4fab9e4d09a2c1a36cbbd2279deb3627 does not match the md5
calculated by the ID: e0094fa02e1334fb67074577464ce5e2 please
update your python configuration or determine the source
of transcription error!
%MSG-w Parameter rho not used: JetCorrFactorsProducer:patJetCorrFactorsPuppi@ctor 19-Oct-2015 14:26:04 CEST pre-events
Module is configured to use the parameter rho, but rho is only used
for L1FastJet corrections. The configuration of levels does not contain
L1FastJet corrections though, so rho will not be used by this module.
%MSG
Reading JEC parameters = SubTotalMC from file = /afs/cern.ch/cms/sw/ReleaseCandidates/vol1/slc6_amd64_gcc493/cms/cmssw-patch/CMSSW_7_6_X_2015-10-18-2300/external/slc6_amd64_gcc4
93/data/CondFormats/JetMETObjects/data/Summer15_50nsV5_DATA_UncertaintySources_AK4PFchs.txt.
%MSG-w HLTTauDQMOfflineSource: HLTTauDQMOfflineSource:hltTauOfflineMonitor_Inclusive@streamBeginRun 19-Oct-2015 14:27:51 CEST Run: 1
HLTTauDQMPath.cc, inferTauLeptonMultiplicity(): module type 'HLTEgammaL1MatchFilterRegional' not recognized, filter 'hltEGL1SingleIsoEG22erOREG25Filter' in path 'HLT_Ele22_eta2p
1_WPLoose_Gsf_LooseIsoPFTau20_v1' will be ignored for offline matching.
%MSG
Changed required number of candidates from 1 to 2 for filter hltL1sL1Mu6DoubleEG10
Could not automatically determine the hadronizer type and set the correct parton selection mode. Parton-based jet flavour will not be defined.
EcalSelectiveReadoutValidation: Invalid iE value: 101
%MSG-e HLTConfigData: PATTriggerProducer:patTrigger 19-Oct-2015 14:30:10 CEST Run: 1 Event: 501
Error in determining HLT prescale set index from L1 data using L1GtUtils: Tech/Phys error = 210110/210110 Tech/Phys psfsi = -1/-1
%MSG
%MSG
%MSG-w TrajectoryStateClosestToPoint_PerigeeConversions: InclusiveVertexFinder:inclusiveVertexFinder 19-Oct-2015 15:21:15 CEST Run: 1 Event: 505
Caught exception An exception of category 'PerigeeConversions' occurred.
Exception Message:
Track with pt=0
.