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

MiniAOD warnings in 12_4_0_pre1 using 2021 data #37227

Closed
tvami opened this issue Mar 13, 2022 · 41 comments
Closed

MiniAOD warnings in 12_4_0_pre1 using 2021 data #37227

tvami opened this issue Mar 13, 2022 · 41 comments

Comments

@tvami
Copy link
Contributor

tvami commented Mar 13, 2022

Modifying the 2021 data based prompt reco wf 138.4 by adding the MiniAOD step, i.e. doing

voms-proxy-init -voms cms -rfc
echo '{"346512" : [[250, 300]]}' > step1_lumiRanges.log
(dasgoclient --limit 0 --query 'lumi,file dataset=/MinimumBias/Commissioning2021-v1/RAW run=346512' --format json | das-selected-lumis.py 250,300 ) | sort -u > step1_dasquery.log
cmsDriver.py step2 --conditions auto:run3_data_prompt -s RAW2DIGI,L1Reco,RECO,PAT --datatier MINIAOD --eventcontent MINIAOD --data --process reRECO --scenario pp --era Run3 --customise Configuration/DataProcessing/RecoTLR.customisePrompt --triggerResultsProcess RECO -n 100 --filein filelist:step1_dasquery.log --lumiToProcess step1_lumiRanges.log --fileout file:step2.root
  • A ROOT problem:
    I get the following warning
input_line_391:10:7: warning: ignoring return value of function declared with 'nodiscard' attribute [-Wunused-result]
      ((const vector<string>*)obj)->empty();
      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  • EGM problem
    There is also a msg about unused products:
%MSG-w UnusedProductsForCanDeleteEarly:  AfterModDestruction  13-Mar-2022 20:22:23 CET pre-events
The following products in the 'canDeleteEarly' list are not used in this job and will be ignored.
 If possible, remove the producer from the job or add the product to the producer's own 'mightGet' list.
 recoIsoDepositedmValueMap_gedElPFIsoDepositChargedAll__reRECO
 recoIsoDepositedmValueMap_gedElPFIsoDepositCharged__reRECO
 recoIsoDepositedmValueMap_gedElPFIsoDepositGamma__reRECO
 recoIsoDepositedmValueMap_gedElPFIsoDepositNeutral__reRECO
 recoIsoDepositedmValueMap_gedElPFIsoDepositPU__reRECO
%MSG
@cmsbuild
Copy link
Contributor

A new Issue was created by @tvami Tamas Vami.

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

cms-bot commands are listed here

@tvami
Copy link
Contributor Author

tvami commented Mar 13, 2022

assign xpog,reconstruction

@cmsbuild
Copy link
Contributor

New categories assigned: xpog

@mariadalfonso,@gouskos,@fgolf you have been requested to review this Pull request/Issue and eventually sign? Thanks

@cmsbuild
Copy link
Contributor

New categories assigned: reconstruction

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

@slava77
Copy link
Contributor

slava77 commented Mar 14, 2022

https://mattermost.web.cern.ch/cms-o-and-c/pl/dy6wygg9mifzpj9nte7eoiz5pa
so, it became an issue now ;)

@jpata
Copy link
Contributor

jpata commented Mar 14, 2022

I cannot see the mattermost discussion above for some reason, can someone copy it here?

I checked that these warnings do not appear in CMSSW_12_3_0_pre1, pre2, pre3, pre4, so one can bisect which release they appeared in.
EDIT: first appears in pre5

At first glance, the first warning (ignoring return value) looks like something gets processed with cling during execution?
EDIT: OK, I see from mattermost that it's the string cut parser...

The second warning (early delete) indicates that gedElPFIsoDeposit are no longer produced, while an early delete is requested.

@jpata
Copy link
Contributor

jpata commented Mar 14, 2022

Potentially related to this PR? #35953
EDIT: no, see #37227 (comment)

@jpata
Copy link
Contributor

jpata commented Mar 14, 2022

The early-delete related warning seems to be from #36882.
As far as I can tell, gedElPFIsoValueChargedAll03 or gedElPFIsoDepositChargedAll is not persisted with the config above.

@tvami
Copy link
Contributor Author

tvami commented Mar 14, 2022

assign core

@cmsbuild
Copy link
Contributor

New categories assigned: core

@Dr15Jones,@smuzaffar,@makortel you have been requested to review this Pull request/Issue and eventually sign? Thanks

@jpata
Copy link
Contributor

jpata commented Mar 14, 2022

As far as I can tell, gedElPFIsoValueChargedAll03 or gedElPFIsoDepositChargedAll is not persisted with the config above.

Note: this does not imply that it's a core issue. Rather, it looks like the cmsDriver config does not keep the gedElPFIsoValueChargedAll03 object, so it's never produced, so the early delete correctly ignores it (if I understood correctly).

@tvami
Copy link
Contributor Author

tvami commented Mar 14, 2022

Note: this does not imply that it's a core issue.

Oh ok, sorry, sure I remove them then?

@makortel
Copy link
Contributor

The early deletion warning indeed came with #36882. As far as I can tell, these producers have not been consumed for a long time, and therefore they have not been run (and been silently deleted since 11_3_0_pre1). #36882 made that visible because their products are being asked to be deleted early in

def customiseEarlyDeleteForCandIsoDeposits(process, products):
# Find the producers
def _branchName(productType, moduleLabel, instanceLabel=""):
return "%s_%s_%s_%s" % (productType, moduleLabel, instanceLabel, process.name_())
for name, module in process.producers_().items():
cppType = module._TypedParameterizable__type
if cppType == "CandIsoDepositProducer":
if module.ExtractorPSet.ComponentName in ["CandViewExtractor", "PFCandWithSuperClusterExtractor"] :
products[name].append(_branchName("recoIsoDepositedmValueMap", name))
return products

@makortel
Copy link
Contributor

@pcanal We're getting the following warning in CMSSW jobs

input_line_391:10:7: warning: ignoring return value of function declared with 'nodiscard' attribute [-Wunused-result]
      ((const vector<string>*)obj)->empty();
      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Back in https://mattermost.web.cern.ch/cms-o-and-c/pl/dy6wygg9mifzpj9nte7eoiz5pa I traced this to

#0  0x00007ffff52b3bbd in write () from /lib64/libc.so.6
...
#8  0x00007fffe03af07a in clang::TextDiagnostic::emitDiagnosticLoc(clang::FullSourceLoc, clang::PresumedLoc, clang::DiagnosticsEngine::Level, llvm::ArrayRef<clang::CharSourceRange>) ()
...
#37 0x00007fffdfe7fc40 in cling::IncrementalParser::ParseInternal(llvm::StringRef) () from /cvmfs/cms-ib.cern.ch/week0/slc7_amd64_gcc10/cms/cmssw/CMSSW_12_3_X_2022-02-15-2300/external/slc7_amd64_gcc10/lib/libCling.so
...
#45 0x00007ffff7e613a7 in edm::FunctionWithDict::FunctionWithDict(TMethod*) () from /cvmfs/cms-ib.cern.ch/week0/slc7_amd64_gcc10/cms/cmssw/CMSSW_12_3_X_2022-02-15-2300/lib/slc7_amd64_gcc10/libFWCoreReflection.so
...
#48 0x00007fffc52c6cd6 in reco::parser::MethodSetter::push()
...
#67 0x00007fffc52e54d4 in reco::parser::cutParser()
#68 0x00007fff3a18fc87 in StringCutObjectSelector<pat::TriggerObjectStandAlone, false>::StringCutObjectSelector(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, bool) ()
#69 0x00007fff3a18ff71 in ObjectSelectorBase<SingleElementCollectionSelector<std::vector<pat::TriggerObjectStandAlone, ...

pointing towards PATTriggerObjectStandAloneSelector. The configuration I looked at the time of that had cut = cms.string('!filterLabels.empty()'). pat::TriggerObjectStandalone::filterLabels() returns const std::vector<std::string> &, matching to the C-style cast on the warning message.

With ROOT in CMSSW_12_3_X_2022-02-15-2300

root [0] std::vector<std::string> foo;
root [1] foo.empty()
ROOT_prompt_1:1:1: warning: ignoring return value of function declared with 'nodiscard' attribute [-Wunused-result]
foo.empty()
^~~~~~~~~
(bool) true

This function call appeared to be the minimal thing causing the warning

funcptr_ = gInterpreter->CallFunc_IFacePtr(callFunc);

Reproduces also with

root [0] auto cl = TClass::GetClass("std::vector<std::string>");
root [1] auto method = (TMethod*)cl->GetListOfMethods()->FindObject("empty");
root [2] TMethodCall call(method);
root [3] auto ptr = call.GetCallFunc();
root [4] auto ptr2 = gInterpreter->CallFunc_IFacePtr(ptr);
input_line_13:10:7: warning: ignoring return value of function declared with 'nodiscard' attribute [-Wunused-result]
      ((const vector<string>*)obj)->empty();
      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
root [5] 

Would you have any suggestions how to silence the warning?

@jpata
Copy link
Contributor

jpata commented Mar 14, 2022

@cms-sw/egamma-pog-l2 please take note and remove gedElPFIsoDeposits if possible from the configs.

@perrotta
Copy link
Contributor

The early-delete related warning seems to be from #36882. As far as I can tell, gedElPFIsoValueChargedAll03 or gedElPFIsoDepositChargedAll is not persisted with the config above.

Event interpretation was removed from RelVal steps with #36465 (in case it may help....)

@pcanal
Copy link
Contributor

pcanal commented Mar 14, 2022

@Axel-Naumann Any idea for this?

@Axel-Naumann
Copy link

@jalopezg-r00t FYI!

@jpata
Copy link
Contributor

jpata commented Mar 15, 2022

Potentially related to this PR? #35953

FWIW pre4 + 35953 does not raise the "ignoring return value of function" warning.

@jpata
Copy link
Contributor

jpata commented Mar 15, 2022

According to the release notes, pre5 introduced root 6.24. As I understand, "ignoring return value of function" is related to that.

@Axel-Naumann
Copy link

Oh, this is using ROOT v6.24?! Fine, we need to backport root-project/root#9244 - @jalopezg-r00t can you do the necessary?

@jalopezg-git
Copy link

Oh, this is using ROOT v6.24?! Fine, we need to backport root-project/root#9244 - @jalopezg-r00t can you do the necessary?

Yes, we were missing a backport to v6.24. PR up here: root-project/root#10120!

@jalopezg-git
Copy link

jalopezg-git commented Mar 16, 2022

@makortel @jpata The required patch has been merged into ROOT's v6-24-00-patches branch some hours ago. Let us know if we can do anything else. 🙂

@jpata
Copy link
Contributor

jpata commented Mar 25, 2022

@smuzaffar just to check - should v6-24-00-patches be propagated to https://github.com/cms-sw/cmsdist/blob/IB/CMSSW_12_4_X/master/root.spec ?

@smuzaffar
Copy link
Contributor

yes

@smuzaffar
Copy link
Contributor

I will take care of it

@smuzaffar
Copy link
Contributor

cms-sw/cmsdist#7722 updates root in 12.4.X to head of v6-24-00-patches

@smuzaffar
Copy link
Contributor

cms-sw/cmsdist#7722 is ready using the tip (commit: 4b08829) of root v6-24-00-patches but I still see the warning

[email protected]>cd /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmsdist/7722/23431/CMSSW_12_4_X_2022-03-27-0000/
[email protected]>cmsenv
**** Following environment variables are going to be unset.
      CMSSW_FULL_RELEASE_BASE

[email protected]>root
   ------------------------------------------------------------------
  | Welcome to ROOT 6.24/07                        https://root.cern |
  | (c) 1995-2021, The ROOT Team; conception: R. Brun, F. Rademakers |
  | Built for linuxx8664gcc on Mar 27 2022, 22:58:00                 |
  | From tag , 2 September 2021                                      |
  | With g++ (GCC) 10.3.0                                            |
  | Try '.help', '.demo', '.license', '.credits', '.quit'/'.q'       |
   ------------------------------------------------------------------

root [0] std::vector<std::string> foo;
root [1] foo.empty()
ROOT_prompt_1:1:1: warning: ignoring return value of function declared with 'nodiscard' attribute [-Wunused-result]
foo.empty()
^~~~~~~~~
(bool) true

@smuzaffar
Copy link
Contributor

by the way, this warning is also present in root master and v6.26 based cmssw IBs

@jalopezg-git
Copy link

I get the following warning

input_line_391:10:7: warning: ignoring return value of function declared with 'nodiscard' attribute [-Wunused-result]
      ((const vector<string>*)obj)->empty();
      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

The applied patch should fix the original issue reported in this ticket (i.e. invoking a [[nodiscard]] function via CallFunc interface does not emit a warning if the result is not used). However, using such function at the prompt still emits a warning if the result is not directly used.

root [0] auto cl = TClass::GetClass("std::vector<std::string>");
root [1] auto method = (TMethod*)cl->GetListOfMethods()->FindObject("empty");
root [2] TMethodCall call(method);
root [3] auto ptr = call.GetCallFunc();
root [4] auto ptr2 = gInterpreter->CallFunc_IFacePtr(ptr);
input_line_13:10:7: warning: ignoring return value of function declared with 'nodiscard' attribute [-Wunused-result]
      ((const vector<string>*)obj)->empty();
      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Can you try this and confirm that it does not generate a diagnostic?

@smuzaffar
Copy link
Contributor

thanks @jalopezg-r00t , I confirm that with latest root v6.24-patches it does not generate the warning.

@smuzaffar
Copy link
Contributor

cms-sw/cmsdist#7722 has been merged and will be available for 11h00 IBs

@tvami
Copy link
Contributor Author

tvami commented Mar 28, 2022

unassign xpog

@SohamBhattacharya
Copy link
Contributor

@cms-sw/egamma-pog-l2 please take note and remove gedElPFIsoDeposits if possible from the configs.

Hi @jpata , could you please clarify if this is still required from E/Gamma, or it's already been fixed?

@tvami
Copy link
Contributor Author

tvami commented Mar 28, 2022

@makortel
Copy link
Contributor

+core

@jpata
Copy link
Contributor

jpata commented Mar 29, 2022

could you please clarify if this is still required from E/Gamma, or it's already been fixed?

@SohamBhattacharya they are still in the configs

gedElPFIsoValueCharged03 = cms.EDProducer("PFCandIsolatorFromDeposits",
, and if Egamma is no longer using them, they should be removed.

@SohamBhattacharya
Copy link
Contributor

could you please clarify if this is still required from E/Gamma, or it's already been fixed?

@SohamBhattacharya they are still in the configs

gedElPFIsoValueCharged03 = cms.EDProducer("PFCandIsolatorFromDeposits",

, and if Egamma is no longer using them, they should be removed.

@jpata Noted, I'll make a PR removing them and also [1] as pointed out by @tvami .

[1] https://github.com/cms-sw/cmssw/blob/master/RecoEgamma/EgammaElectronProducers/python/electronPFIsolationDeposits_cff.py#L24-L37

@jpata
Copy link
Contributor

jpata commented Apr 27, 2022

+reconstruction

Thanks all involved!

@cmsbuild
Copy link
Contributor

This issue is fully signed and ready to be closed.

@tvami
Copy link
Contributor Author

tvami commented Apr 27, 2022

Thanks everybody involved, I'm closing the issue now!

@tvami tvami closed this as completed Apr 27, 2022
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