residual plots for DQM #17645
residual plots for DQM #17645
Conversation
|
A new Pull Request was created by @aspiezia for CMSSW_9_0_X. It involves the following packages: DQM/SiStripMonitorClient @cmsbuild, @dmitrijus, @vanbesien, @davidlange6 can you please review it and eventually sign? Thanks. cms-bot commands are listed here #13028 |
|
+1 |
cmsbuild
added
dqm-approved
fully-signed
and removed
dqm-pending
pending-signatures
labels
|
The tests are being triggered in jenkins. |
|
This pull request is fully signed and it will be integrated in one of the next CMSSW_9_0_X IBs after it passes the integration tests. This pull request requires discussion in the ORP meeting before it's merged. @davidlange6, @smuzaffar |
|
Comparison job queued. |
| std::string topFolderName_ = "SiStrip"; | ||
| SiStripFolderOrganizer folder_organizer; | ||
| folder_organizer.setSiStripFolderName(topFolderName_); | ||
| tkhisto_ResidualsMean = new TkHistoMap(ibooker , topFolderName_ ,"TkHMap_ResidualsMean", 0.0,true); |
There was a problem hiding this comment.
@aspiezia - do these need to be deleted? Better to make them unique_ptrs
There was a problem hiding this comment.
HI, I cannot find the end of job where I can delete the object. I have tried to look for some examples in cmssw and I cannot find a place where these tkhisto maps are deleted (see for example https://github.com/aspiezia/cmssw/blob/a0a714e6c65870a27ee5a346d1d0e75367573276/DQM/SiStripMonitorTrack/src/SiStripMonitorTrack.cc#L153). Do you have a suggestion?
There was a problem hiding this comment.
Hi @aspiezia,
forget about deleting those at the end job and do what @davidlange6 suggested - use unique_ptrs.
Modify https://github.com/aspiezia/cmssw/blob/a0a714e6c65870a27ee5a346d1d0e75367573276/DQM/SiStripMonitorTrack/interface/SiStripMonitorTrack.h#L131 and change TkHistoMap *x to std::unique_ptr<TkHistoMap> x;
There was a problem hiding this comment.
Hi @dmitrijus and @davidlange6, thanks a lot for the suggestion.
Consider that https://github.com/aspiezia/cmssw/blob/a0a714e6c65870a27ee5a346d1d0e75367573276/DQM/SiStripMonitorTrack/interface/SiStripMonitorTrack.h#L131 is not the code I am modifying. It was just an example showing that everywhere in cmssw, the tracker maps are defined as I am doing.
Anyway, I have tried your suggestion in the code I am modifing and it does not compile because of this line: https://github.com/aspiezia/cmssw/blob/a0a714e6c65870a27ee5a346d1d0e75367573276/DQM/SiStripMonitorTrack/interface/SiStripMonitorTrack.h#L131
Do you have a suggestion on how to change it?
Thanks,
Aniello
|
-1 |
|
I have removed the "TkHMap_ResidualsRMS" that was a copy of "TkHMap_ResidualsMean" and I have added the RMS map in DQM/SiStripMonitorClient/src/SiStripTrackerMapCreator.cc as suggested by @fioriNTU |
|
i'm not sure I understand the question? You have some choices- a)if you need to use bare pointers for some reason (I can't think of one), then anything you new has to be deleted when you are done with the object. |
|
On Mar 8, 2017, at 11:50 AM, aspiezia ***@***.***> wrote:
@aspiezia commented on this pull request.
In DQM/TrackerMonitorTrack/src/MonitorTrackResiduals.cc:
> @@ -38,6 +38,11 @@ void MonitorTrackResidualsBase<pixel_or_strip>::bookHistograms(DQMStore::IBooker
m_cacheID_ = cacheID;
this->createMEs( ibooker , iSetup);
}
+ std::string topFolderName_ = "SiStrip";
+ SiStripFolderOrganizer folder_organizer;
+ folder_organizer.setSiStripFolderName(topFolderName_);
+ tkhisto_ResidualsMean = new TkHistoMap(ibooker , topFolderName_ ,"TkHMap_ResidualsMean", 0.0,true);
Hi @dmitrijus and @davidlange6, thanks a lot for the suggestion.
Consider that https://github.com/aspiezia/cmssw/blob/a0a714e6c65870a27ee5a346d1d0e75367573276/DQM/SiStripMonitorTrack/interface/SiStripMonitorTrack.h#L131 is not the code I am modifying. It was just an example showing that everywhere in cmssw, the tracker maps are defined as I am doing.
Right, looks like an old bug that should be addressed
Anyway, I have tried your suggestion in the code I am modifing and it does not compile because of this line: https://github.com/aspiezia/cmssw/blob/a0a714e6c65870a27ee5a346d1d0e75367573276/DQM/SiStripMonitorTrack/interface/SiStripMonitorTrack.h#L131
Do you have a suggestion on how to change it?
I think your link is incorrect?
…
Thanks,
Aniello
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.
|
|
Right @davidlange6, |
|
yes, you need to follow the right syntax (easy to ask google) - something like
tkhisto_ResidualsMean = std::unique_ptr<TkHistoMap>(new TkHistoMap(ibooker , topFolderName_ ,"TkHMap_ResidualsMean", 0.0,true));
… On Mar 8, 2017, at 12:01 PM, aspiezia ***@***.***> wrote:
Right @davidlange6,
this is the correct link:
https://github.com/aspiezia/cmssw/blob/86068d7740011458e797555f7ef745502f349271/DQM/TrackerMonitorTrack/src/MonitorTrackResiduals.cc#L44
Aniello
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.
|
|
Pull request #17645 was updated. @cmsbuild, @dmitrijus, @vanbesien, @davidlange6 can you please check and sign again. |
|
Thanks a lot for the suggestion. The change has been implemented. |
|
Pull request #17645 was updated. @cmsbuild, @dmitrijus, @vanbesien, @davidlange6 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 CMSSW_9_0_X IBs after it passes the integration tests. This pull request requires discussion in the ORP meeting before it's merged. @davidlange6, @smuzaffar |
|
Comparison job queued. |
|
+1 |
Mean and RMS of the residuals of the tracks have been added in the DQM. Details can be found in this presentation:
https://indico.cern.ch/event/536893/contributions/2389697/attachments/1388650/2114441/AnielloSpiezia_141216.pdf