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

new BPH reco packages #15484

Merged
merged 43 commits into from Oct 27, 2016
Merged

new BPH reco packages #15484

merged 43 commits into from Oct 27, 2016

Conversation

ronchese
Copy link
Contributor

New packages to reconstruct B hadrons decays encapsulating loops over particles, selections, vertex and kinematic fit, discussed with BPH conveners and PAG.

Package HeavyFlavor/RecoDecay contains code for basic operations (including an example)

Package HeavyFlavor/SpecificDecay contains code implementing common particle selections (based on pt, eta...) and reconstruction of most common heavy flavoured particles (Psi,Upsilon,B+,B0,Bs)
An example is included, and a module to add the reconstructed particles to the Event is included as well.

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @ronchese for CMSSW_8_1_X.

It involves the following packages:

HeavyFlavorAnalysis/RecoDecay
HeavyFlavorAnalysis/SpecificDecay

The following packages do not have a category, yet:

HeavyFlavorAnalysis/RecoDecay
HeavyFlavorAnalysis/SpecificDecay

@cmsbuild, @davidlange6 can you please review it and eventually sign? Thanks.
@slava77, @smuzaffar you are the release manager for this.

cms-bot commands are list here #13028

@slava77
Copy link
Contributor

slava77 commented Aug 16, 2016

@ronchese please point to some slides with details about the proposed code additions.

Please clarify where will this code run typically.

  • just some specific BPH analysis jobs
  • standard processing of all BPH-related data in AOD or miniAOD (as a part of SKIM or AlcaRECO definition)

Who is going to support these 11K lines of code on the time scale of a few years.

I think we will need a presentation in RECO/AT meeting with details on implementation.
I didn't look at the code in much detail, but there appear to be some repeated patterns. If any substantive copy-pasted code is present, it will have to be reduced.

@slava77
Copy link
Contributor

slava77 commented Aug 16, 2016

@cmsbuild please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Aug 16, 2016

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

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

@slava77
Copy link
Contributor

slava77 commented Aug 16, 2016

@ronchese
Please address issues mentioned in the code static analyzer report
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-15484/14554/llvm-analysis/

There is also an issue with
HeavyFlavorAnalysis/SpecificDecay/src/classes_def.xml

  • it defines dictionaries for types that are not defined in this package itself
  • it defines persistent products outside of DataFormats subsystem. This can be done only for transient test products.
    These dictionaries should be defined elsewhere, an appropriate place appears to be DataFormats/PatCandidates/src/classes_user.h

@ronchese
Copy link
Contributor Author

Hi

Thank you a lot for your message.

About the maintenace:
I developed these packages on request from the BPH PAG; of course
I can take care of the maintenance of the packages at least for the
first period (1 or 2 years); for long term I suppose any decision is
to be taken in the BPH PAG (the conveners will receive this message in CC).

About the usage:
The main aim of these packages is providing some tool reconstructing
quarkonia and B decays to ease starting new analyses; then this could be
possibly included in the skimming procedure to save in the skimmed events
the informations about candidates for some basic decays for BPH analyses.

About the presentation:
Please tell me which time slots can be considered, (I'm trying to have
just a bit of holidays in this period...), in the meanwhile you can have
a look at a presentation (in a BPH meeting) of a preliminary version of
the RecoDecay package, with the basic tools to reconstruct any decay
imagined by the user:

https://indico.cern.ch/event/403474/contributions/960950/attachments/808357/1107845/commonReco_150623.pdf

A documentation twiki was also set up:

https://twiki.cern.ch/twiki/bin/view/CMS/BPHReco

Since then several improvements have been introduced in the RecoDecay
package, but not yet documented (to be done as soon as the situation is
stable); main difference vs. the description in the twiki is that the
output objects are given as smart pointers (shared_ptr) in place of
native C++ pointers.
The package "SpecificDecay" is somehow new and simply implements the
reconstruction of quarkonia, B+, B0 and Bs.

About code replication:
I usually avoid all code replication I can; I'm aware of code parts similar
(but not identical) but apart of that it's possible that in the development
something got duplicated. Exactly which parts are you referring to?

Thanks

Paolo

On Tue, 16 Aug 2016, Slava Krutelyov wrote:

@ronchese please point to some slides with details about the proposed code
additions.

Please clarify where will this code run typically.

  • just some specific BPH analysis jobs
  • standard processing of all BPH-related data in AOD or miniAOD (as a part
    of SKIM or AlcaRECO definition)

Who is going to support these 11K lines of code on the time scale of a few
years.

I think we will need a presentation in RECO/AT meeting with details on
implementation.
I didn't look at the code in much detail, but there appear to be some
repeated patterns. If any substantive copy-pasted code is present, it will
have to be reduced.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the
thread.[AE4A-ea9BoDZfT--0SdYmMrICzCj-IW1ks5qgfFVgaJpZM4JlnI8.gif]

@ronchese
Copy link
Contributor Author

OK, I'll fix all that a.s.a.p.
By the way, once fixed and committed, must I reissue the pull request?
It's some time I'm not developing code for CMSSW, and last time it was
still CVS and tag-collector, so I'm not yet acquainted with git and
pull-requests.

Thanks,

Paolo

On Tue, 16 Aug 2016, Slava Krutelyov wrote:

@ronchese
Please address issues mentioned in the code static analyzer report
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-15
484/14554/llvm-analysis/

There is also an issue with
HeavyFlavorAnalysis/SpecificDecay/src/classes_def.xml

  • it defines dictionaries for types that are not defined in this package
    itself
  • it defines persistent products outside of DataFormats subsystem. This
    can be done only for transient test products. These dictionaries should
    be defined elsewhere, an appropriate place appears to be
    DataFormats/PatCandidates/src/classes_user.h


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the
thread.[AE4A-TEjSoigmHU7-68_DnynGyy_TkfQks5qgj4SgaJpZM4JlnI8.gif]

<use name="TrackingTools/PatternTools"/>
<use name="TrackingTools/Records"/>
<use name="TrackingTools/TransientTrack"/>
<flags CXXFLAGS="-Wno-delete-non-virtual-dtor"/>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CXXFLAGS use should be justified.
suppressing a warning doesn't look like a good solution

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this was added to remove warnings coming from other packages and making
harder to spot warnings from my code...

On Tue, 23 Aug 2016, Slava Krutelyov wrote:

In HeavyFlavorAnalysis/RecoDecay/BuildFile.xml:

+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+

CXXFLAGS use should be justified.
suppressing a warning doesn't look like a good solution


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the
thread.[AE4A-WnPZeb69EX6ufdkW-ariJcb_dAEks5qixYhgaJpZM4JlnI8.gif]

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

which other packages?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not remember, but apparently the problem disappeared.
Probably it has been fixed since my first test with older CMSSW

On Tue, 23 Aug 2016, Slava Krutelyov wrote:

In HeavyFlavorAnalysis/RecoDecay/BuildFile.xml:

+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+

which other packages?


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the
thread.[AE4A-Qd1bvpC5fFHHr40QwH6UAcsDHhmks5qiyC8gaJpZM4JlnI8.gif]

@cmsbuild
Copy link
Contributor

Comparison not run due to Build errors (RelVals and Igprof tests were also skipped)

@cmsbuild
Copy link
Contributor

-1

Tested at: 1ce854d

You can see the results of the tests here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-15484/15617/summary.html

I found follow errors while testing this PR

Failed tests: Build ClangBuild

  • Build:

I found an error when building:

>> Compiling  /build/cmsbld/jenkins-workarea/workspace/ib-any-integration/CMSSW_8_1_X_2016-10-10-1100/src/HeavyFlavorAnalysis/SpecificDecay/src/BPHOniaToMuMuBuilder.cc 
>> Compiling  /build/cmsbld/jenkins-workarea/workspace/ib-any-integration/CMSSW_8_1_X_2016-10-10-1100/src/HeavyFlavorAnalysis/SpecificDecay/src/BPHKx0ToKPiBuilder.cc 
>> Compiling edm plugin /build/cmsbld/jenkins-workarea/workspace/ib-any-integration/CMSSW_8_1_X_2016-10-10-1100/src/HeavyFlavorAnalysis/SpecificDecay/plugins/BPHHistoSpecificDecay.cc 
>> Compiling edm plugin /build/cmsbld/jenkins-workarea/workspace/ib-any-integration/CMSSW_8_1_X_2016-10-10-1100/src/HeavyFlavorAnalysis/SpecificDecay/plugins/BPHWriteSpecificDecay.cc 
>> Compiling edm plugin /build/cmsbld/jenkins-workarea/workspace/ib-any-integration/CMSSW_8_1_X_2016-10-10-1100/src/HeavyFlavorAnalysis/SpecificDecay/test/stubs/TestBPHSpecificDecay.cc 
/build/cmsbld/jenkins-workarea/workspace/ib-any-integration/CMSSW_8_1_X_2016-10-10-1100/src/HeavyFlavorAnalysis/SpecificDecay/src/BPHKx0ToKPiBuilder.cc:21:74: fatal error: HeavyFlavorAnalysis/SpecificDecay/interface/BPHMassSymSelect.h: No such file or directory
compilation terminated.
/build/cmsbld/jenkins-workarea/workspace/ib-any-integration/CMSSW_8_1_X_2016-10-10-1100/src/HeavyFlavorAnalysis/SpecificDecay/src/BPHKx0ToKPiBuilder.cc:21:74: fatal error: HeavyFlavorAnalysis/SpecificDecay/interface/BPHMassSymSelect.h: No such file or directory
compilation terminated.
gmake: **\* [tmp/slc6_amd64_gcc530/src/HeavyFlavorAnalysis/SpecificDecay/src/HeavyFlavorAnalysisSpecificDecay/BPHKx0ToKPiBuilder.o] Error 1
>> Compiling edm plugin /build/cmsbld/jenkins-workarea/workspace/ib-any-integration/CMSSW_8_1_X_2016-10-10-1100/src/HeavyFlavorAnalysis/SpecificDecay/test/stubs/CheckBPHWriteDecay.cc 

  • Clang:

I found a compilation error while trying to compile with clang:
I used this command:
scram b vclean && scram build -k -j 16 USER_CXXFLAGS='-fsyntax-only' COMPILER='llvm compile'

                   ^~~~~~~~~~~~~~~~
/cvmfs/cms-ib.cern.ch/2016-42/slc6_amd64_gcc530/external/gcc/5.3.0/lib/gcc/x86_64-pc-linux-gnu/5.3.0/../../../../include/c++/5.3.0/bits/atomic_base.h:157:26: note: expanded from macro 'ATOMIC_FLAG_INIT'
#define ATOMIC_FLAG_INIT { 0 }
                         ^~~~~
1 warning generated.
/build/cmsbld/jenkins-workarea/workspace/ib-any-integration/CMSSW_8_1_X_2016-10-10-1100/src/HeavyFlavorAnalysis/SpecificDecay/src/BPHKx0ToKPiBuilder.cc:21:10: fatal error: 'HeavyFlavorAnalysis/SpecificDecay/interface/BPHMassSymSelect.h' file not found
#include "HeavyFlavorAnalysis/SpecificDecay/interface/BPHMassSymSelect.h"
         ^
1 error generated.
/build/cmsbld/jenkins-workarea/workspace/ib-any-integration/CMSSW_8_1_X_2016-10-10-1100/src/HeavyFlavorAnalysis/SpecificDecay/src/BPHKx0ToKPiBuilder.cc:21:10: fatal error: 'HeavyFlavorAnalysis/SpecificDecay/interface/BPHMassSymSelect.h' file not found
#include "HeavyFlavorAnalysis/SpecificDecay/interface/BPHMassSymSelect.h"


@monttj
Copy link
Contributor

monttj commented Oct 10, 2016

@ronchese

It looks like there is one more file missing.
HeavyFlavorAnalysis/SpecificDecay/interface/BPHMassSymSelect.h

Taejeong

@cmsbuild
Copy link
Contributor

Pull request #15484 was updated. @cmsbuild, @monttj, @davidlange6 can you please check and sign again.

@monttj
Copy link
Contributor

monttj commented Oct 10, 2016

@cmsbuild please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Oct 10, 2016

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

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

@monttj
Copy link
Contributor

monttj commented Oct 11, 2016

+1
@ronchese
Major comments were implemented. I hope in next PR for maintenance, some remaining comments will be taken into account.

@cmsbuild
Copy link
Contributor

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

@davidlange6
Copy link
Contributor

+1

@cmsbuild cmsbuild merged commit 4320586 into cms-sw:CMSSW_8_1_X Oct 27, 2016
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

5 participants