Removed self assignment of variable in GlobalTrackQualityProducer #25741

Dr15Jones · 2019-01-22T22:03:31Z

This fixes a clang warning.

cmsbuild · 2019-01-22T22:03:53Z

The code-checks are being triggered in jenkins.

cmsbuild · 2019-01-22T22:09:37Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-25741/8136

This PR adds an extra 16KB to repository

cmsbuild · 2019-01-22T22:10:08Z

A new Pull Request was created by @Dr15Jones (Chris Jones) for master.

It involves the following packages:

RecoMuon/GlobalTrackingTools

@cmsbuild, @perrotta, @slava77 can you please review it and eventually sign? Thanks.
@bellan, @abbiendi, @jhgoh, @echapon, @calderona, @HuguesBrun, @folguera, @battibass, @trocino, @amagitte, @bachtis, @rociovilar 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

Dr15Jones · 2019-01-22T22:12:25Z

please test

cmsbuild · 2019-01-22T22:12:53Z

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/32782/console Started: 2019/01/22 23:13

cmsbuild · 2019-01-23T00:43:09Z

+1
Tested at: 618657c
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-25741/32782/summary.html

cmsbuild · 2019-01-23T00:43:15Z

Comparison job queued.

cmsbuild · 2019-01-23T09:25:44Z

Comparison is ready
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-25741/32782/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: 3097440
DQMHistoTests: Total failures: 1
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3097242
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

cmsbuild · 2019-01-23T14:39:02Z

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)

slava77 · 2019-01-23T14:51:29Z

RecoMuon/GlobalTrackingTools/plugins/GlobalTrackQualityProducer.cc

@@ -285,11 +285,9 @@ std::pair<double,double> GlobalTrackQualityProducer::newChi2(Trajectory& muon) c
    }
  }

-  if (tkNdof < 6 ) tkChi2 = tkChi2; // or should I set it to  a large number ?
-  else tkChi2 /= (tkNdof-5.);
+  if (tkNdof > 5 ) {tkChi2 /= (tkNdof-5.); }


is it really obvious that tkChi2 = tkChi2; // or should I set it to a large number ? was not a typo?

From 'blame' that line has been in the code for 10 years.

https://github.com/cms-sw/cmssw/blame/master/RecoMuon/GlobalTrackingTools/plugins/GlobalTrackQualityProducer.cc#L288

I "refreshed" my memory a bit.
This part was questionable to me back when it was proposed.
At least for historical reason, I propose to leave a comment here about the other values; smth like if (tkNdof < 6 ) the tkChi2 is used directly: should a large number or something else be used perhaps with a FIXME.

@drkovalskyi is anybody still using these IDs?

cmsbuild · 2019-01-23T16:33:25Z

The code-checks are being triggered in jenkins.

cmsbuild · 2019-01-23T16:39:24Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-25741/8148

This PR adds an extra 16KB to repository

Dr15Jones · 2019-01-23T16:39:42Z

please test

cmsbuild · 2019-01-23T16:39:50Z

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/32798/console Started: 2019/01/23 17:40

cmsbuild · 2019-01-23T16:39:52Z

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

cmsbuild · 2019-01-23T18:52:23Z

+1
Tested at: e3c4b31
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-25741/32798/summary.html

cmsbuild · 2019-01-23T18:52:29Z

Comparison job queued.

cmsbuild · 2019-01-23T20:40:10Z

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

Comparison Summary:

No significant changes to the logs found
Reco comparison results: 14 differences found in the comparisons
DQMHistoTests: Total files compared: 32
DQMHistoTests: Total histograms compared: 3097440
DQMHistoTests: Total failures: 1
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3097242
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 · 2019-01-23T23:27:16Z

+1

for #25741 e3c4b31

code changes are in line with the PR description and the follow up review; the change is technical
jenkins tests pass and comparisons with the baseline show no relevant differences
- the last tests executed in CMSSW_10_5_X_2019-01-23-1100 had some unrelated differences in the wf 25.0
- logically equivalent version of the code tested in CMSSW_10_5_X_2019-01-22-1100 showed no differences

cmsbuild · 2019-01-23T23:27:40Z

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 · 2019-01-24T09:46:56Z

+1

Removed self assignment of variable in GlobalTrackQualityProducer

618657c

This fixes a clang warning.

cmsbuild added this to the CMSSW_10_5_X milestone Jan 22, 2019

cmsbuild added code-checks-pending comparison-pending orp-pending pending-signatures reconstruction-pending tests-pending labels Jan 22, 2019

cmsbuild added code-checks-approved and removed code-checks-pending labels Jan 22, 2019

cmsbuild added tests-started and removed tests-pending labels Jan 22, 2019

cmsbuild added tests-approved and removed tests-started labels Jan 23, 2019

cmsbuild added comparison-available fully-signed reconstruction-approved and removed comparison-pending pending-signatures reconstruction-pending labels Jan 23, 2019

slava77 reviewed Jan 23, 2019

View reviewed changes

cmsbuild added the tests-pending label Jan 23, 2019

cmsbuild added code-checks-approved and removed code-checks-pending labels Jan 23, 2019

cmsbuild added tests-started and removed tests-pending labels Jan 23, 2019

cmsbuild added tests-approved and removed tests-started labels Jan 23, 2019

cmsbuild added comparison-available and removed comparison-pending labels Jan 23, 2019

cmsbuild added fully-signed reconstruction-approved and removed pending-signatures reconstruction-pending labels Jan 23, 2019

perrotta mentioned this pull request Jan 24, 2019

Remove unused instance of TrajectoryStateTransform #25750

Merged

cmsbuild added orp-approved and removed orp-pending labels Jan 24, 2019

cmsbuild merged commit 9242d8e into cms-sw:master Jan 24, 2019

Dr15Jones deleted the clangGlobalTrackQualityProducer branch January 24, 2019 14:28

slava77 mentioned this pull request Feb 8, 2022

Gem turn off demo unpacker - 12_2_X backport of #36769 #36910

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Removed self assignment of variable in GlobalTrackQualityProducer #25741

Removed self assignment of variable in GlobalTrackQualityProducer #25741

Dr15Jones commented Jan 22, 2019

cmsbuild commented Jan 22, 2019

cmsbuild commented Jan 22, 2019

cmsbuild commented Jan 22, 2019

Dr15Jones commented Jan 22, 2019

cmsbuild commented Jan 22, 2019 •

edited

cmsbuild commented Jan 23, 2019

cmsbuild commented Jan 23, 2019

cmsbuild commented Jan 23, 2019

cmsbuild commented Jan 23, 2019

slava77 Jan 23, 2019

Dr15Jones Jan 23, 2019

Dr15Jones Jan 23, 2019

slava77 Jan 23, 2019

cmsbuild commented Jan 23, 2019

cmsbuild commented Jan 23, 2019

Dr15Jones commented Jan 23, 2019

cmsbuild commented Jan 23, 2019 •

edited

cmsbuild commented Jan 23, 2019

cmsbuild commented Jan 23, 2019

cmsbuild commented Jan 23, 2019

cmsbuild commented Jan 23, 2019

slava77 commented Jan 23, 2019

cmsbuild commented Jan 23, 2019

fabiocos commented Jan 24, 2019

Removed self assignment of variable in GlobalTrackQualityProducer #25741

Removed self assignment of variable in GlobalTrackQualityProducer #25741

Conversation

Dr15Jones commented Jan 22, 2019

cmsbuild commented Jan 22, 2019

cmsbuild commented Jan 22, 2019

cmsbuild commented Jan 22, 2019

Dr15Jones commented Jan 22, 2019

cmsbuild commented Jan 22, 2019 • edited

cmsbuild commented Jan 23, 2019

cmsbuild commented Jan 23, 2019

cmsbuild commented Jan 23, 2019

cmsbuild commented Jan 23, 2019

slava77 Jan 23, 2019

Choose a reason for hiding this comment

Dr15Jones Jan 23, 2019

Choose a reason for hiding this comment

Dr15Jones Jan 23, 2019

Choose a reason for hiding this comment

slava77 Jan 23, 2019

Choose a reason for hiding this comment

cmsbuild commented Jan 23, 2019

cmsbuild commented Jan 23, 2019

Dr15Jones commented Jan 23, 2019

cmsbuild commented Jan 23, 2019 • edited

cmsbuild commented Jan 23, 2019

cmsbuild commented Jan 23, 2019

cmsbuild commented Jan 23, 2019

cmsbuild commented Jan 23, 2019

slava77 commented Jan 23, 2019

cmsbuild commented Jan 23, 2019

fabiocos commented Jan 24, 2019

cmsbuild commented Jan 22, 2019 •

edited

cmsbuild commented Jan 23, 2019 •

edited