Skip to content
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

TrackerMap script not working in CMSSW_11_3_2 #34607

Closed
sroychow opened this issue Jul 23, 2021 · 16 comments
Closed

TrackerMap script not working in CMSSW_11_3_2 #34607

sroychow opened this issue Jul 23, 2021 · 16 comments

Comments

@sroychow
Copy link
Contributor

Tracker DQM uses a standalone script to produce 2-Dimensional plots of various monitored quantities. Example of TkMaps for a run taken in 2018(link). The script in turn runs the SiStripOfflineDQM module. The script takes as input legacy TDirectory based DQM_*.root file.

Currently the script is crashing in CMSSW_11_3_2 when we try to run it for producing the maps for the CRUZET runs.
It crashes from line 205 which is calling the DQMStore::open function. The crash comes from this line. From my understanding, the support to read TDirectory based ROOT files were dropped in this PR #28622.

To solve the issue, I tried implementing a function to read the contents of the TDirectory based ROOT file with the LegacyIOhelper class, but not all maps are being filled correctly. I am still investigating. Can the DQM experts give some suggestions?
@arossi83 @mmusich @tsusa

@cmsbuild
Copy link
Contributor

A new Issue was created by @sroychow Suvankar Roy Chowdhury.

@Dr15Jones, @perrotta, @dpiparo, @silviodonato, @smuzaffar, @makortel, @qliphy can you please review it and eventually sign/assign? Thanks.

cms-bot commands are listed here

@makortel
Copy link
Contributor

assign dqm

@cmsbuild
Copy link
Contributor

New categories assigned: dqm

@jfernan2,@kmaeshima,@rvenditti,@andrius-k,@ErnestaP,@ahmad3213 you have been requested to review this Pull request/Issue and eventually sign? Thanks

@mmusich
Copy link
Contributor

mmusich commented Jul 26, 2021

@cms-sw/dqm-l2 since the functionality has been removed by the central DQM team, please advice on how to proceed. This tool is needed for CRUZET data certification. Thank you

@jfernan2
Copy link
Contributor

Could
CommonTools/UtilAlgos/interface/TFileService.h
and/or
CommonTools/Utils/interface/TFileDirectory.h
be a solution for you?

@mmusich
Copy link
Contributor

mmusich commented Jul 26, 2021

@sroychow see please #34607 (comment)

@sroychow
Copy link
Contributor Author

@jfernan2 the solution of TFileService only solves the part of reading the files contents. However this doesn't ensure that the DQM store object is filled with the proper Monitor Eelement info. Here e.g., I have tried implementing the file reading(this can be optimized of-course) the legacyIO helper class - link. But then in this version, the TkMaps related to QTests are not getting filled.

@jfernan2
Copy link
Contributor

So, you need read and write Legacy access, not only read, I misunderstood then. I am sorry for the inconvenience of this issue

The problem is that you generally cannot use analyzers with the legacy (plain root) saver, so, e.g. normal DQM modules running in a process with a DQMSaver are expected not to produce output, because they expect to work on runs, while the DQMSaver saves at endjob.

A possible solution (not sure how difficult may be to hack this) would be to use TFileService to read and in parallel DQMFileSaver to write copying the MEs from input to output and adding your needs in between.

For this use case subsystems generally use plain .C scripts outside the CMSSW/DQM FW, but I understand that the TkQualityMaps make this harder.

@sroychow could you please remind me what particularities do TkMaps have? Aren't they TH2F/D objects? Aren't they filled or saved in your trial with LegacyIOHelper? I see they are not explicitelty considered in:

// INTs are saved as strings in this format: <objectName>i=value</objectName>
// REALs are saved as strings in this format: <objectName>f=value</objectName>
// STRINGs are saved as strings in this format: <objectName>s="value"</objectName>
if (me->kind() == MonitorElement::Kind::INT) {
int value = me->getIntValue();
std::string content = "<" + objectName + ">i=" + std::to_string(value) + "</" + objectName + ">";
TObjString str(content.c_str());
str.Write();
} else if (me->kind() == MonitorElement::Kind::REAL) {
double value = me->getFloatValue();
char buf[64];
// use printf here to preserve exactly the classic formatting.
std::snprintf(buf, sizeof(buf), "%.*g", DBL_DIG + 2, value);
std::string content = "<" + objectName + ">f=" + buf + "</" + objectName + ">";
TObjString str(content.c_str());
str.Write();
} else if (me->kind() == MonitorElement::Kind::STRING) {
const std::string &value = me->getStringValue();
std::string content = "<" + objectName + ">s=" + value + "</" + objectName + ">";
TObjString str(content.c_str());
str.Write();
} else {
// Write a histogram
TH1 *value = me->getTH1();
value->Write();
if (me->getEfficiencyFlag()) {
std::string content = "<" + objectName + ">e=1</" + objectName + ">";
TObjString str(content.c_str());
str.Write();
}
for (QReport *qr : me->getQReports()) {
std::string result;
// TODO: 64 is likely too short; memory corruption in the old code?
char buf[64];
std::snprintf(buf, sizeof(buf), "qr=st:%d:%.*g:", qr->getStatus(), DBL_DIG + 2, qr->getQTresult());
result = '<' + objectName + '.' + qr->getQRName() + '>';
result += buf;
result += qr->getAlgorithm() + ':' + qr->getMessage();
result += "</" + objectName + '.' + qr->getQRName() + '>';
TObjString str(result.c_str());
str.Write();
}
}

Because that could be the reason why they are not saved properly

Thanks

@mmusich
Copy link
Contributor

mmusich commented Aug 17, 2021

One of the problems of the approach at #34607 (comment) is that running it takes an inordinate amount of time to run to completion.
It is not traversing the input DQM file the problem, but rather the creation of the TrackerMaps themselves.
Running:

igprof -pp -z -o ig.gz -t cmsRun cmsRun ${CMSSW_BASE}/src/DQM/SiStripMonitorClient/test/SiStripDQM_OfflineTkMap_Template_cfg_DB.py globalTag=113X_dataRun3_Prompt_v3 runNumber=319176 dqmFile=DQM_V0001_R000319176__ZeroBias__Run2018B-PromptReco-v2__DQMIO.root detIdInfoFile=file.root
igprof-analyse --sqlite -v --demangle --gdb ig.gz | sed -e 's/INSERT INTO files VALUES (\([^,]*\), \"[^$]*/INSERT INTO files VALUES (\1, \"ABCD\");/g' | sqlite3 ig.sql3

I get the following results: http://musich.web.cern.ch/musich/cgi-bin/igprof-navigator/ig_tkmaps/12

image

so it seems that > 98% of the time is passed in the destructor of TPolyLine.
Apparently removing the delete of the TPolyLines at:

for (std::vector<TPolyLine *>::iterator pos1 = vp.begin(); pos1 != vp.end(); pos1++) {
delete (*pos1);
}

renders the execution reasonably fast, but will obviously leak memory (as the new is not deleted).
Making them smart pointers doesn't seem to help either.
Does anyone have a good suggestion on that?

@Dr15Jones
Copy link
Contributor

Instead of doing new and delete all the time, you could reuse the poly line by resetting its value via
https://root.cern.ch/doc/master/classTPolyLine.html#af9615d068b67a0d6bf9e1bba4ea3d83b

@Dr15Jones
Copy link
Contributor

Looking at the implementation of SetPolyLine it doesn't reuse the underlying storage so it is just like doing a delete followed by a new. Only SetPoint appears to reuse the storage.

@Dr15Jones
Copy link
Contributor

Ah, so it looks like calling
https://root.cern.ch/doc/master/classTPolyLine.html#ad3e1ccd7ced0116da830612a342b411c
followed by a series of SetPoint would reuse the storage.

@mmusich
Copy link
Contributor

mmusich commented Aug 18, 2021

I tried out the SetPolyLine option at 4831754
but while it makes it fast, it does produce empty maps: http://musich.web.cern.ch/musich/display/TrackerMapsFixed/

@mmusich
Copy link
Contributor

mmusich commented Oct 11, 2021

this can be signed and closed, right?

@jfernan2
Copy link
Contributor

+1

@cmsbuild
Copy link
Contributor

This issue is fully signed and ready to be closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants