added 3D and beamspot/vertex options #11660

fojensen · 2015-10-06T18:34:04Z

would like this implemented in CMSSW_7_5_X

added a few options:

select which beamSpot to work with
use a vertex collection instead of the beamspot as a reference point
added cut on the significance of the impact parameter in the Z direction of the daughter tracks (default is off)
allow one to use a 3d instead of a 2d pointing angle cut
allow one to use a 3d instead of a 2d decay length cut

here are plots of a few standard quantities for the V0s before and after the changes to the producer:
http://www-hep.colorado.edu/~fjensen/06102015_V0pullrequest/
created with
/RelValMinBias_13/CMSSW_7_5_0-75X_mcRun2_asymptotic_v1-v1/GEN-SIM-DIGI-RAW-HLTDEBUG

cmsbuild · 2015-10-06T18:36:12Z

A new Pull Request was created by @fojensen (Frank Jensen) for CMSSW_7_5_X.

added 3D and beamspot/vertex options

It involves the following packages:

RecoVertex/V0Producer

@cmsbuild, @cvuosalo, @slava77 can you please review it and eventually sign? Thanks.
@makortel, @GiacomoSguazzoni, @rovere, @VinInn, @cerati, @dgulhan this is something you requested to watch as well.
You can sign-off by replying to this message having '+1' in the first line of your reply.
You can reject by replying to this message having '-1' in the first line of your reply.
If you are a L2 or a release manager you can ask for tests by saying 'please test' in the first line of a comment.
@Degano you are the release manager for this.
You can merge this pull request by typing 'merge' in the first line of your comment.

slava77 · 2015-10-06T18:45:27Z

RecoVertex/V0Producer/src/V0Fitter.h

-
-#include <string>
-#include <fstream>
+#include "CommonTools/CandUtils/interface/AddFourMomenta.h"


better have all the includes in the .cc file: based on the changes in this file, these extra includes are not needed at the class declaration step.

sorry this is new to me - is the general idea to have an include statement
in the header file (i.e. class declaration file) only if it is necessary in
the declaration of the class?

In my particular example, the class declaration involves a function with a
parameter of reco::VertexCompositeCandidateCollection - in this case I
would want to have the particular include file in the same file as the
class declaration?

On Tue, Oct 6, 2015 at 2:45 PM, Slava Krutelyov notifications@github.com
wrote:

In RecoVertex/V0Producer/src/V0Fitter.h
#11660 (comment):

#include "DataFormats/TrackingRecHit/interface/TrackingRecHit.h"

#include "DataFormats/BeamSpot/interface/BeamSpot.h"

-#include
-#include
+#include "CommonTools/CandUtils/interface/AddFourMomenta.h"

better have all the includes in the .cc file: based on the changes in this
file, these extra includes are not needed at the class declaration step.

—
Reply to this email directly or view it on GitHub
https://github.com/cms-sw/cmssw/pull/11660/files#r41305162.

I didn't mean to suggest removal of all includes: only most of the new ones.

The diffs in this file have only one new non-trivial member change: reco::Vertex, which is already included.

All new includes, like MessageLogger, TrajectoryStateTransform, TSCBLBuilderNoMaterial don't appear to be necessary in the class interface declaration and should be moved to the implementation file.

a somewhat related question:
if I notice that the code compiles WITHOUT a certain include statement,
such as
#include "FWCore/MessageLogger/interface/MessageLogger.h"
is it generally safe to leave out entirely?

On Tue, Oct 6, 2015 at 3:23 PM, Slava Krutelyov notifications@github.com
wrote:

In RecoVertex/V0Producer/src/V0Fitter.h
#11660 (comment):

#include "DataFormats/TrackingRecHit/interface/TrackingRecHit.h"

#include "DataFormats/BeamSpot/interface/BeamSpot.h"

-#include
-#include
+#include "CommonTools/CandUtils/interface/AddFourMomenta.h"

I didn't mean to suggest removal of all includes: only most of the new
ones.

The diffs in this file have only one new non-trivial member change:
reco::Vertex, which is already included.

All new includes, like MessageLogger, TrajectoryStateTransform,
TSCBLBuilderNoMaterial don't appear to be necessary in the class interface
declaration and should be moved to the implementation file.

—
Reply to this email directly or view it on GitHub
https://github.com/cms-sw/cmssw/pull/11660/files#r41309868.

it's better not to rely on nested non-obvious includes and add an include (for a MessageLogger) in the .cc in this case where the corresponding methods (LogError etc) are used.

slava77 · 2015-10-06T18:53:07Z

@cmsbuild please test

cmsbuild · 2015-10-06T18:56:09Z

The tests are being triggered in jenkins.

cmsbuild · 2015-10-06T19:46:10Z

Pull request #11660 was updated. @cmsbuild, @cvuosalo, @slava77 can you please check and sign again.

cmsbuild · 2015-10-06T20:23:41Z

-1
Tested at: 9751feb
When I ran the RelVals I found an error in the following worklfows:
25202.0 step2

runTheMatrix-results/25202.0_TTbar_13+TTbar_13+DIGIUP15_PU25+RECOUP15_PU25+HARVESTUP15_PU25/step2_TTbar_13+TTbar_13+DIGIUP15_PU25+RECOUP15_PU25+HARVESTUP15_PU25.log
----- Begin Fatal Exception 06-Oct-2015 21:38:59 CEST-----------------------
An exception of category 'FallbackFileOpenError' occurred while
   [0] Constructing the EventProcessor
   [1] Constructing module: class=MixingModule label='mix'
   [2] Calling RootFileSequenceBase::initTheFile()
   [3] Calling StorageFactory::open()
   [4] Calling XrdFile::open()
Exception Message:
Failed to open the file 'root://xrootd-cms.infn.it//store/relval/CMSSW_7_5_0/RelValMinBias_13/GEN-SIM/75X_mcRun2_asymptotic_v1-v1/00000/76F6801B-362A-E511-930F-0025905A60B4.root'
   Additional Info:
      [a] XrdCl::File::Open(name='root://eoscms//eos/cms/store/relval/CMSSW_7_5_0/RelValMinBias_13/GEN-SIM/75X_mcRun2_asymptotic_v1-v1/00000/76F6801B-362A-E511-930F-0025905A60B4.root?svcClass=default', flags=0x10, permissions=0660) => error '[ERROR] Server responded with an error: [3011] Unable to open file /eos/cms/store/relval/CMSSW_7_5_0/RelValMinBias_13/GEN-SIM/75X_mcRun2_asymptotic_v1-v1/00000/76F6801B-362A-E511-930F-0025905A60B4.root; No such file or directory
' (errno=3011, code=400). No additional data servers were found.
      [b] Last URL tried: root://eoscms:1094//eos/cms/store/relval/CMSSW_7_5_0/RelValMinBias_13/GEN-SIM/75X_mcRun2_asymptotic_v1-v1/00000/76F6801B-362A-E511-930F-0025905A60B4.root?svcClass=default&tried=
      [c] Problematic data server: eoscms:1094
      [d] Disabled source: eoscms:1094
      [e] Input file root://eoscms//eos/cms/store/relval/CMSSW_7_5_0/RelValMinBias_13/GEN-SIM/75X_mcRun2_asymptotic_v1-v1/00000/76F6801B-362A-E511-930F-0025905A60B4.root?svcClass=default could not be opened.
Fallback Input file root://xrootd-cms.infn.it//store/relval/CMSSW_7_5_0/RelValMinBias_13/GEN-SIM/75X_mcRun2_asymptotic_v1-v1/00000/76F6801B-362A-E511-930F-0025905A60B4.root also could not be opened.
Original exception info is above; fallback exception info is below.
      [f] XrdCl::File::Open(name='root://xrootd-cms.infn.it//store/relval/CMSSW_7_5_0/RelValMinBias_13/GEN-SIM/75X_mcRun2_asymptotic_v1-v1/00000/76F6801B-362A-E511-930F-0025905A60B4.root', flags=0x10, permissions=0660) => error '[FATAL] Redirect limit has been reached' (errno=0, code=306). No additional data servers were found.
      [g] Last URL tried: root://cmsxrootd-site2.fnal.gov:1093//store/relval/CMSSW_7_5_0/RelValMinBias_13/GEN-SIM/75X_mcRun2_asymptotic_v1-v1/00000/76F6801B-362A-E511-930F-0025905A60B4.root?tried=+1213xrootd.ba.infn.it,
      [h] Problematic data server: cmsxrootd-site2.fnal.gov:1093
      [i] Disabled source: cmsxrootd-site2.fnal.gov:1093
----- End Fatal Exception -------------------------------------------------

50202.0 step2

runTheMatrix-results/50202.0_TTbar_13+TTbar_13+DIGIUP15_PU50+RECOUP15_PU50+HARVESTUP15_PU50/step2_TTbar_13+TTbar_13+DIGIUP15_PU50+RECOUP15_PU50+HARVESTUP15_PU50.log
----- Begin Fatal Exception 06-Oct-2015 21:39:35 CEST-----------------------
An exception of category 'FallbackFileOpenError' occurred while
   [0] Constructing the EventProcessor
   [1] Constructing module: class=MixingModule label='mix'
   [2] Calling RootFileSequenceBase::initTheFile()
   [3] Calling StorageFactory::open()
   [4] Calling XrdFile::open()
Exception Message:
Failed to open the file 'root://xrootd-cms.infn.it//store/relval/CMSSW_7_5_0/RelValMinBias_13/GEN-SIM/75X_mcRun2_asymptotic_v1-v1/00000/148E8070-332A-E511-B3B5-0025905B8598.root'
   Additional Info:
      [a] XrdCl::File::Open(name='root://eoscms//eos/cms/store/relval/CMSSW_7_5_0/RelValMinBias_13/GEN-SIM/75X_mcRun2_asymptotic_v1-v1/00000/148E8070-332A-E511-B3B5-0025905B8598.root?svcClass=default', flags=0x10, permissions=0660) => error '[ERROR] Server responded with an error: [3011] Unable to open file /eos/cms/store/relval/CMSSW_7_5_0/RelValMinBias_13/GEN-SIM/75X_mcRun2_asymptotic_v1-v1/00000/148E8070-332A-E511-B3B5-0025905B8598.root; No such file or directory
' (errno=3011, code=400). No additional data servers were found.
      [b] Last URL tried: root://eoscms:1094//eos/cms/store/relval/CMSSW_7_5_0/RelValMinBias_13/GEN-SIM/75X_mcRun2_asymptotic_v1-v1/00000/148E8070-332A-E511-B3B5-0025905B8598.root?svcClass=default&tried=
      [c] Problematic data server: eoscms:1094
      [d] Disabled source: eoscms:1094
      [e] Input file root://eoscms//eos/cms/store/relval/CMSSW_7_5_0/RelValMinBias_13/GEN-SIM/75X_mcRun2_asymptotic_v1-v1/00000/148E8070-332A-E511-B3B5-0025905B8598.root?svcClass=default could not be opened.
Fallback Input file root://xrootd-cms.infn.it//store/relval/CMSSW_7_5_0/RelValMinBias_13/GEN-SIM/75X_mcRun2_asymptotic_v1-v1/00000/148E8070-332A-E511-B3B5-0025905B8598.root also could not be opened.
Original exception info is above; fallback exception info is below.
      [f] XrdCl::File::Open(name='root://xrootd-cms.infn.it//store/relval/CMSSW_7_5_0/RelValMinBias_13/GEN-SIM/75X_mcRun2_asymptotic_v1-v1/00000/148E8070-332A-E511-B3B5-0025905B8598.root', flags=0x10, permissions=0660) => error '[FATAL] Redirect limit has been reached' (errno=0, code=306). No additional data servers were found.
      [g] Last URL tried: root://cmsxrootd-site1.fnal.gov:1093//store/relval/CMSSW_7_5_0/RelValMinBias_13/GEN-SIM/75X_mcRun2_asymptotic_v1-v1/00000/148E8070-332A-E511-B3B5-0025905B8598.root?tried=+1213xrootd.ba.infn.it,
      [h] Problematic data server: cmsxrootd-site1.fnal.gov:1093
      [i] Disabled source: cmsxrootd-site1.fnal.gov:1093
----- End Fatal Exception -------------------------------------------------

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

slava77 · 2015-10-07T00:08:50Z

Frank,
sorry, I didn't pay attention to the branch, which is 75X.

At this point we allow updates in 75X only for HeavyIon processing needs (this constraint has formalized in the last few days).
Your change should go to 76X.
Please prepare a PR for 76X.

I tried to make a diff CMSSW_7_6_X...fojensen:V0forHLT with 76X and it looks like you can reuse the same branch and make the new PR directly from the web (no need to make a new branch)

fojensen · 2015-10-07T03:20:10Z

Hey Slava,

This actually is for heavy ion needs - I was contacted by a group at Rice
to make these changes.

~Frank

On Tue, Oct 6, 2015 at 8:08 PM, Slava Krutelyov notifications@github.com
wrote:

Frank,
sorry, I didn't pay attention to the branch, which is 75X.

At this point we allow updates in 75X only for HeavyIon processing needs
(this constraint has formalized in the last few days).
Your change should go to 76X.
Please prepare a PR for 76X.

I tried to make a diff CMSSW_7_6_X...fojensen:V0forHLT
CMSSW_7_6_X...fojensen:V0forHLT
with 76X and it looks like you can reuse the same branch and make the new
PR directly from the web (no need to make a new branch)

—
Reply to this email directly or view it on GitHub
#11660 (comment).

cmsbuild · 2015-10-07T03:56:13Z

Pull request #11660 was updated. @cmsbuild, @cvuosalo, @slava77 can you please check and sign again.

slava77 · 2015-10-07T12:16:55Z

@cmsbuild please test

cmsbuild · 2015-10-07T12:21:09Z

The tests are being triggered in jenkins.

cmsbuild · 2015-10-07T20:46:33Z

Pull request #11660 was updated. @cmsbuild, @cvuosalo, @slava77 can you please check and sign again.

slava77 · 2015-10-07T20:56:55Z

@cmsbuild please test

cmsbuild · 2015-10-07T21:01:35Z

The tests are being triggered in jenkins.

cmsbuild · 2015-10-07T22:26:40Z

+1
Tested at: 2b5afcb
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-11660/8677/summary.html

cmsbuild · 2015-10-07T23:07:16Z

Comparison is ready
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-11660/8677/summary.html

slava77 · 2015-10-07T23:40:39Z

+1

for #11660 2b5afcb

new options/features added to the V0 reconstruction: they are off in current configuration
jenkins tests pass and comparisons with baseline show no differences, as expected
additional tests done locally in CMSSW_7_5_3_patch1 confirm no difference and no significant change in CPU use in the V0 reconstruction

Frank, please make a 76X pull request.
I think the same branch can work there as well.
Thank you.

cmsbuild · 2015-10-07T23:41:16Z

This pull request is fully signed and it will be integrated in one of the next CMSSW_7_5_X IBs (tests are also fine) and once validation in the development release cycle CMSSW_7_6_X is complete. This pull request requires discussion in the ORP meeting before it's merged. @davidlange6, @Degano, @smuzaffar

davidlange6 · 2015-10-20T17:13:39Z

+1

added 3D and beamspot/vertex options

added 3D and beamspot/vertex options

9751feb

cmsbuild added this to the Next CMSSW_7_5_X milestone Oct 6, 2015

cmsbuild added reconstruction-pending pending-signatures tests-pending orp-pending comparison-pending labels Oct 6, 2015

slava77 reviewed Oct 6, 2015
View reviewed changes

cmsbuild added tests-started and removed tests-pending labels Oct 6, 2015

put some includes back in the .cc file

1cbeab9

cmsbuild added tests-pending and removed tests-started labels Oct 6, 2015

cmsbuild added tests-rejected and removed tests-pending labels Oct 6, 2015

calculate angles using dot product

ffa6679

cmsbuild added tests-pending and removed tests-rejected labels Oct 7, 2015

cmsbuild removed the tests-pending label Oct 7, 2015

use beamspot instead of beamspot position

2b5afcb

cmsbuild added tests-pending and removed tests-rejected labels Oct 7, 2015

cmsbuild added tests-started and removed tests-pending labels Oct 7, 2015

cmsbuild added tests-approved and removed tests-started labels Oct 7, 2015

cmsbuild added comparison-available and removed comparison-pending labels Oct 7, 2015

cmsbuild added reconstruction-approved fully-signed and removed pending-signatures reconstruction-pending labels Oct 7, 2015

fojensen mentioned this pull request Oct 8, 2015

add 75x V0Producer to 76x #11699

Merged

This was referenced Oct 18, 2015

add 75x V0Producer to 74x #11865

Closed

push V0Producer from 75X and 76X into 74X #11974

Merged

cmsbuild added orp-approved and removed orp-pending labels Oct 20, 2015

cmsbuild added a commit that referenced this pull request Oct 20, 2015

Merge pull request #11660 from fojensen/V0forHLT

69009e1

added 3D and beamspot/vertex options

cmsbuild merged commit 69009e1 into cms-sw:CMSSW_7_5_X Oct 20, 2015

fojensen mentioned this pull request Oct 28, 2015

add vertex covariance for V0Producer 76x #12142

Merged

slava77 mentioned this pull request Oct 28, 2015

allow vertex covariance #12133

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

added 3D and beamspot/vertex options #11660

added 3D and beamspot/vertex options #11660

fojensen commented Oct 6, 2015

cmsbuild commented Oct 6, 2015

slava77 Oct 6, 2015

fojensen Oct 6, 2015

slava77 Oct 6, 2015

fojensen Oct 6, 2015

slava77 Oct 6, 2015

slava77 commented Oct 6, 2015

cmsbuild commented Oct 6, 2015

cmsbuild commented Oct 6, 2015

cmsbuild commented Oct 6, 2015

slava77 commented Oct 7, 2015

fojensen commented Oct 7, 2015

cmsbuild commented Oct 7, 2015

slava77 commented Oct 7, 2015

cmsbuild commented Oct 7, 2015

cmsbuild commented Oct 7, 2015

slava77 commented Oct 7, 2015

cmsbuild commented Oct 7, 2015

cmsbuild commented Oct 7, 2015

cmsbuild commented Oct 7, 2015

slava77 commented Oct 7, 2015

cmsbuild commented Oct 7, 2015

davidlange6 commented Oct 20, 2015

added 3D and beamspot/vertex options #11660

added 3D and beamspot/vertex options #11660

Conversation

fojensen commented Oct 6, 2015

cmsbuild commented Oct 6, 2015

slava77 Oct 6, 2015

Choose a reason for hiding this comment

fojensen Oct 6, 2015

Choose a reason for hiding this comment

#include "DataFormats/BeamSpot/interface/BeamSpot.h"

slava77 Oct 6, 2015

Choose a reason for hiding this comment

fojensen Oct 6, 2015

Choose a reason for hiding this comment

#include "DataFormats/BeamSpot/interface/BeamSpot.h"

slava77 Oct 6, 2015

Choose a reason for hiding this comment

slava77 commented Oct 6, 2015

cmsbuild commented Oct 6, 2015

cmsbuild commented Oct 6, 2015

cmsbuild commented Oct 6, 2015

slava77 commented Oct 7, 2015

fojensen commented Oct 7, 2015

cmsbuild commented Oct 7, 2015

slava77 commented Oct 7, 2015

cmsbuild commented Oct 7, 2015

cmsbuild commented Oct 7, 2015

slava77 commented Oct 7, 2015

cmsbuild commented Oct 7, 2015

cmsbuild commented Oct 7, 2015

cmsbuild commented Oct 7, 2015

slava77 commented Oct 7, 2015

cmsbuild commented Oct 7, 2015

davidlange6 commented Oct 20, 2015