Extend TrackBase and Track to include time and velocity #24465

lgray · 2018-09-04T17:01:37Z

Add time, velocity, and corresponding diagonal covariance terms to reco::TrackBase.
These (in addition to #24419) are the minimal changes needed for MTD development.

This is purely technical, getting it in place so that we can ease development.

cmsbuild · 2018-09-04T17:01:54Z

The code-checks are being triggered in jenkins.

cmsbuild · 2018-09-04T17:03:35Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-24465/6305

cmsbuild · 2018-09-04T17:03:55Z

A new Pull Request was created by @lgray (Lindsey Gray) for master.

It involves the following packages:

DataFormats/GsfTrackReco
DataFormats/TrackReco

@perrotta, @cmsbuild, @slava77 can you please review it and eventually sign? Thanks.
@makortel, @gpetruc, @rovere, @VinInn this is something you requested to watch as well.
@davidlange6, @slava77, @fabiocos you are the release manager for this.

cms-bot commands are listed here

lgray · 2018-09-04T17:16:48Z

@cmsbuild please test

cmsbuild · 2018-09-04T17:18:01Z

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/30259/console Started: 2018/09/04 19:18

cmsbuild · 2018-09-04T23:31:52Z

+1
Tested at: 043c249
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-24465/30259/summary.html

cmsbuild · 2018-09-04T23:31:55Z

Comparison job queued.

cmsbuild · 2018-09-05T00:56:19Z

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

Comparison Summary:

No significant changes to the logs found
Reco comparison results: 0 differences found in the comparisons
DQMHistoTests: Total files compared: 32
DQMHistoTests: Total histograms compared: 3143879
DQMHistoTests: Total failures: 1
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3143681
DQMHistoTests: Total skipped: 197
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 31 files compared)
Checked 133 log files, 14 edm output root files, 32 DQM output files

slava77 · 2018-09-06T21:54:39Z

DataFormats/TrackReco/interface/TrackBase.h

-              signed char nloops = 0, uint8_t stopReason = 0);
+              signed char nloops = 0, uint8_t stopReason = 0,
+	      double t0 = 0, double beta = 0, 
+	      double covt0t0 = -1., double covbetabeta = -1.);


please match the indentation

it looks like the new arguments should be floats (with correspondingly float defaults)

slava77 · 2018-09-06T21:57:14Z

DataFormats/TrackReco/interface/Track.h

-          TrackAlgorithm = undefAlgorithm, TrackQuality quality = undefQuality);
+          TrackAlgorithm = undefAlgorithm, TrackQuality quality = undefQuality,
+	  double t0 = 0, double beta = 0, 
+	  double covt0t0 = -1., double covbetabeta = -1.);


please match the indentation

it looks like the new arguments should be floats (with correspondingly float defaults)

Hmm the indentation was matched in my text editor....

Changed to double... Ditto for the other similar comment.

It's the "usual" mixture of tabs and spaces, works with one tab width but not others.

slava77 · 2018-09-06T21:57:47Z

DataFormats/TrackReco/interface/Track.h

@@ -50,7 +52,7 @@ class Track : public TrackBase
    bool innerOk() const {
        return extra_->innerOk();
    }
-
+    


the empty space change is not necessary

slava77 · 2018-09-06T23:04:43Z

DataFormats/TrackReco/interface/TrackBase.h

+// Velocity at the reference point on the track
+inline double TrackBase::beta() const
+{
+    return beta_;


Is this an absolute velocity, or is it normalized to c? Please clarify in the comments.

slava77 · 2018-09-06T23:26:07Z

DataFormats/TrackReco/interface/TrackBase.h

+// error on beta
+inline double TrackBase::betaError() const
+{
+    return std::sqrt(covbetabeta_);


there should also be a way to get these without sqrt.
Perhaps covarianceT0T0 and covarianceBetaBeta.
BTW, are the two strictly related by a track-dependent constant or is there a T0Beta term?

slava77 · 2018-09-06T23:26:22Z

DataFormats/TrackReco/src/Track.cc

+	     double covt0t0, double covbetabeta) :
+  TrackBase(chi2, ndof, vertex, momentum, charge, cov, algo, quality,
+	    0,0, // nloops and stop reason
+	    t0,beta,covt0t0,covbetabeta)


indentation

slava77 · 2018-09-06T23:26:32Z

DataFormats/TrackReco/src/TrackBase.cc

@@ -89,10 +89,15 @@ TrackBase::TrackBase() :

 TrackBase::TrackBase(double chi2, double ndof, const Point &vertex, const Vector &momentum,
                     int charge, const CovarianceMatrix &cov, TrackAlgorithm algorithm,
-                     TrackQuality quality, signed char nloops, uint8_t stopReason):
+                     TrackQuality quality, signed char nloops, uint8_t stopReason,
+		     double t0, double beta, double covt0t0, double covbetabeta):


indentation

slava77 · 2018-09-06T23:33:40Z

DataFormats/TrackReco/interface/TrackBase.h

+
+    /// norm of the particle velocity at innermost point on track
+    /// can multiply by momentum_.Unit() to get velocity vector
+    float beta_;


please define default values here or in the default constructor.
I can't recall now if they also should be specified in the ioread rules (it's easy to check by reading an old file and checking what's in covt0t0_ )

slava77 · 2018-09-13T13:42:12Z

@lgray
please clarify on the plans for updating the PR to follow up on the code review.
Thank you.

lgray · 2018-09-14T20:07:15Z

I'll update this by the end of today.

slava77 · 2018-09-20T17:39:38Z

I'll update this by the end of today.

@lgray
please clarify on the plans for updating the PR to follow up on the code review.
thank you.

lgray · 2018-09-24T17:49:42Z

@slava77 working on it now!

cmsbuild · 2018-09-25T03:01:38Z

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

Comparison Summary:

No significant changes to the logs found
Reco comparison results: 2 differences found in the comparisons
DQMHistoTests: Total files compared: 32
DQMHistoTests: Total histograms compared: 3146876
DQMHistoTests: Total failures: 3
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3146676
DQMHistoTests: Total skipped: 197
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 31 files compared)
Checked 133 log files, 14 edm output root files, 32 DQM output files

slava77 · 2018-09-28T17:32:07Z

+1

for #24465 4419bc4

code changes are in line with the PR description and the follow up review
jenkins tests pass and comparisons with the baseline show no [relevant] differences
local tests confirm that the default values are correctly propagated when reading old data files

cmsbuild · 2018-09-28T17:32:26Z

This pull request is fully signed and it will be integrated in one of the next master IBs (tests are also fine). This pull request will now be reviewed by the release team before it's merged. @davidlange6, @slava77, @smuzaffar, @fabiocos (and backports should be raised in the release meeting by the corresponding L2)

fabiocos · 2018-10-02T08:10:57Z

please test workflow 22434.0

cmsbuild · 2018-10-02T08:11:13Z

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/30836/console Started: 2018/10/02 10:11

cmsbuild · 2018-10-02T12:47:59Z

+1
Tested at: 4419bc4
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-24465/30836/summary.html

cmsbuild · 2018-10-02T12:48:02Z

Comparison job queued.

cmsbuild · 2018-10-02T14:25:41Z

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

Comparison Summary:

No significant changes to the logs found
Reco comparison results: 2 differences found in the comparisons
DQMHistoTests: Total files compared: 32
DQMHistoTests: Total histograms compared: 3162800
DQMHistoTests: Total failures: 2
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3162601
DQMHistoTests: Total skipped: 197
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 31 files compared)
Checked 134 log files, 14 edm output root files, 32 DQM output files

fabiocos · 2018-10-10T20:01:51Z

+1

extend TrackBase and Track to include time and velocity

043c249

cmsbuild added this to the CMSSW_10_3_X milestone Sep 4, 2018

cmsbuild added code-checks-pending comparison-pending orp-pending pending-signatures reconstruction-pending tests-pending labels Sep 4, 2018

cmsbuild added code-checks-approved and removed code-checks-pending labels Sep 4, 2018

cmsbuild added tests-started and removed tests-pending labels Sep 4, 2018

cmsbuild added tests-approved and removed tests-started labels Sep 4, 2018

cmsbuild added comparison-available and removed comparison-pending labels Sep 5, 2018

slava77 reviewed Sep 6, 2018

View reviewed changes

cmsbuild removed the code-checks-approved label Sep 24, 2018

cmsbuild added comparison-available and removed comparison-pending labels Sep 25, 2018

cmsbuild added fully-signed reconstruction-approved and removed pending-signatures reconstruction-pending labels Sep 28, 2018

cmsbuild added comparison-pending tests-pending and removed comparison-available tests-approved labels Oct 2, 2018

cmsbuild added tests-started and removed tests-pending labels Oct 2, 2018

cmsbuild added tests-approved and removed tests-started labels Oct 2, 2018

cmsbuild added comparison-available and removed comparison-pending labels Oct 2, 2018

smuzaffar modified the milestones: CMSSW_10_3_X, CMSSW_10_4_X Oct 10, 2018

cmsbuild added orp-approved and removed orp-pending labels Oct 10, 2018

cmsbuild merged commit 059e627 into cms-sw:master Oct 10, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Extend TrackBase and Track to include time and velocity #24465

Extend TrackBase and Track to include time and velocity #24465

lgray commented Sep 4, 2018 •

edited

cmsbuild commented Sep 4, 2018

cmsbuild commented Sep 4, 2018

cmsbuild commented Sep 4, 2018

lgray commented Sep 4, 2018

cmsbuild commented Sep 4, 2018 •

edited

cmsbuild commented Sep 4, 2018

cmsbuild commented Sep 4, 2018

cmsbuild commented Sep 5, 2018

slava77 Sep 6, 2018

slava77 Sep 6, 2018

lgray Sep 24, 2018

makortel Sep 24, 2018

slava77 Sep 6, 2018

slava77 Sep 6, 2018

slava77 Sep 6, 2018

slava77 Sep 6, 2018

slava77 Sep 6, 2018

slava77 Sep 6, 2018

slava77 commented Sep 13, 2018

lgray commented Sep 14, 2018

slava77 commented Sep 20, 2018

lgray commented Sep 24, 2018

cmsbuild commented Sep 25, 2018

slava77 commented Sep 28, 2018

cmsbuild commented Sep 28, 2018

fabiocos commented Oct 2, 2018

cmsbuild commented Oct 2, 2018 •

edited

cmsbuild commented Oct 2, 2018

cmsbuild commented Oct 2, 2018

cmsbuild commented Oct 2, 2018

fabiocos commented Oct 10, 2018

Extend TrackBase and Track to include time and velocity #24465

Extend TrackBase and Track to include time and velocity #24465

Conversation

lgray commented Sep 4, 2018 • edited

cmsbuild commented Sep 4, 2018

cmsbuild commented Sep 4, 2018

cmsbuild commented Sep 4, 2018

lgray commented Sep 4, 2018

cmsbuild commented Sep 4, 2018 • edited

cmsbuild commented Sep 4, 2018

cmsbuild commented Sep 4, 2018

cmsbuild commented Sep 5, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

slava77 commented Sep 13, 2018

lgray commented Sep 14, 2018

slava77 commented Sep 20, 2018

lgray commented Sep 24, 2018

cmsbuild commented Sep 25, 2018

slava77 commented Sep 28, 2018

cmsbuild commented Sep 28, 2018

fabiocos commented Oct 2, 2018

cmsbuild commented Oct 2, 2018 • edited

cmsbuild commented Oct 2, 2018

cmsbuild commented Oct 2, 2018

cmsbuild commented Oct 2, 2018

fabiocos commented Oct 10, 2018

lgray commented Sep 4, 2018 •

edited

cmsbuild commented Sep 4, 2018 •

edited

cmsbuild commented Oct 2, 2018 •

edited