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
The use of the innerTrack is now protected with an if, also a cout ha… #23015
Conversation
…s been removed, and the logic of a beamspot isvalid has been corrected
The code-checks are being triggered in jenkins. |
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-23015/4413 |
A new Pull Request was created by @parbol for master. It involves the following packages: DQMOffline/Muon @vazzolini, @kmaeshima, @dmitrijus, @cmsbuild, @jfernan2, @vanbesien can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
h_allProbes_inner_phi->Fill(muon2->innerTrack()->innerPosition().Phi()); | ||
|
||
if(muon2->innerTrack().isNonnull()) { | ||
h_allProbes_inner_pt->Fill(muon2->innerTrack()->innerMomentum().Rho()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this will not help.
Did you check that the crash is gone?
innerTrack().extra().isAvailable()
looks like a more correct approach
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi,
I couldn't check it was working, because I was unable to run the workflow that was failing. I tried your recommendations and more things but for this particular workflow I just didn't manage to run (it was crashing for unrelated reasons). I can update this right away, but I'm afraid that I won't be able to check it. I'm happy to try any other suggestion you have.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@parbol we need to use innerTrack().extra().isAvailable(). isNonnull() doesn't mean that data is available (I know it's confusing if you don't use it each day, but it covers different use cases).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have corrected this now. Thanks for the hint!
The code-checks are being triggered in jenkins. |
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-23015/4416 |
Pull request #23015 was updated. @vazzolini, @kmaeshima, @dmitrijus, @cmsbuild, @jfernan2, @vanbesien can you please check and sign again. |
+1 |
The tests are being triggered in jenkins. |
This pull request is fully signed and it will be integrated in one of the next master IBs after it passes the integration tests. 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) |
Comparison job queued. |
Comparison is ready Comparison Summary:
|
+1 |
Backport of #23015 to 101X, DQMOffline/Muon fixes
This PR corrects an issue discussed here:
#22456 (comment)
In particular it corrects three things:
The innerTrack is tested to be "isnonNull()" before being used, otherwise the code is not executed.
The logic of the beamspot spotted before is now corrected to uses beamSpot().isValid()
One remaining cout has been replaced by a edm::Log