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.

Sign up for GitHub

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

Jump to bottom

Proposed solution to ECAL emulator PR issue #35557

Merged
merged 12 commits into from Oct 14, 2021
Merged

Proposed solution to ECAL emulator PR issue #35557

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
1798695
Changed ECAL emulator debug and TPinfo cout print statements to LogVe…
atishelmanch Sep 28, 2021
52ae7b0
Added string pointer destruction, updated log info a bit
atishelmanch Sep 29, 2021
6628373
Cleaning up and improving EcalTPG verbatim statements
atishelmanch Sep 29, 2021
d2c82e6
Combining a few EcalTPG LogVerbatim lines
atishelmanch Sep 29, 2021
0fefcf4
Rethrowing EcalCondDB and CondTool catches as cms Exceptions
atishelmanch Oct 1, 2021
db012a6
Reverted test executables, and replaced couts with loginfo
atishelmanch Oct 4, 2021
1936292
CMSSW code checks and code format
atishelmanch Oct 4, 2021
1cee3ab
Replaced many EcalTPG loginfo statements with logverbatim statements
atishelmanch Oct 5, 2021
5960e9e
Reran CMSSW code checks and code format
atishelmanch Oct 5, 2021
e986346
Updated ECAL TPGmode printout formatting, introduced either smart poi…
atishelmanch Oct 8, 2021
2b68e21
Replaced one extra exit statement with a cms exception
atishelmanch Oct 8, 2021
912bc18
Removing ECAL delete statements to revert to known working version. A…
atishelmanch Oct 11, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 0 additions & 1 deletion CondFormats/EcalObjects/src/EcalTPGTPMode.cc
View file
Open in desktop
@@ -1,6 +1,5 @@
#include "CondFormats/EcalObjects/interface/EcalTPGTPMode.h"
#include <iostream>
#include "FWCore/MessageLogger/interface/MessageLogger.h"

EcalTPGTPMode::EcalTPGTPMode() {}

Expand Down
109 changes: 52 additions & 57 deletions CondTools/Ecal/src/EcalTPGOddWeightIdMapHandler.cc
View file
Open in desktop
Expand Up @@ -45,35 +45,34 @@ void popcon::EcalTPGOddWeightIdMapHandler::getNewObjects() {
} else if (m_file_type == "xml") {
readxmlFile();
} else {
edm::LogInfo("EcalTPGOddWeightIdMapHandler") << "Started GetNewObjects!!!";
edm::LogVerbatim("EcalTPGOddWeightIdMapHandler") << "Started GetNewObjects!!!";

//check whats already inside of database
if (tagInfo().size) {
//check whats already inside of database
std::cout << "got offlineInfo = " << std::endl;
std::cout << "tag name = " << tagInfo().name << std::endl;
std::cout << "size = " << tagInfo().size << std::endl;
edm::LogVerbatim("EcalTPGOddWeightIdMapHandler") << "got offlineInfo = ";
edm::LogVerbatim("EcalTPGOddWeightIdMapHandler") << "tag name = " << tagInfo().name;
edm::LogVerbatim("EcalTPGOddWeightIdMapHandler") << "size = " << tagInfo().size;

} else {
std::cout << " First object for this tag " << std::endl;
edm::LogVerbatim("EcalTPGOddWeightIdMapHandler") << " First object for this tag ";
}

unsigned int max_since = 0;
max_since = static_cast<unsigned int>(tagInfo().lastInterval.since);
edm::LogInfo("EcalTPGOddWeightIdMapHandler") << "max_since : " << max_since;
//Ref weightIdMap_db = lastPayload();
edm::LogVerbatim("EcalTPGOddWeightIdMapHandler") << "max_since : " << max_since;

edm::LogInfo("EcalTPGOddWeightIdMapHandler") << "retrieved last payload ";
edm::LogVerbatim("EcalTPGOddWeightIdMapHandler") << "retrieved last payload ";

// here we retrieve all the runs after the last from online DB
edm::LogInfo("EcalTPGOddWeightIdMapHandler") << "Retrieving run list from ONLINE DB ... ";
edm::LogVerbatim("EcalTPGOddWeightIdMapHandler") << "Retrieving run list from ONLINE DB ... ";

edm::LogInfo("EcalTPGOddWeightIdMapHandler") << "Making connection...";
edm::LogVerbatim("EcalTPGOddWeightIdMapHandler") << "Making connection...";
econn = new EcalCondDBInterface(m_sid, m_user, m_pass);
edm::LogInfo("EcalTPGOddWeightIdMapHandler") << "Done.";
edm::LogVerbatim("EcalTPGOddWeightIdMapHandler") << "Done.";

if (!econn) {
std::cout << " connection parameters " << m_sid << "/" << m_user << std::endl;
// cerr << e.what() << std::endl;
edm::LogVerbatim("EcalTPGOddWeightIdMapHandler") << " connection parameters " << m_sid << "/" << m_user;
throw cms::Exception("OMDS not available");
}

Expand Down Expand Up @@ -101,29 +100,28 @@ void popcon::EcalTPGOddWeightIdMapHandler::getNewObjects() {
if (min_run < max_since) {
min_run = max_since + 1; // we have to add 1 to the last transferred one
}

std::cout << "m_i_run_number" << m_i_run_number << "m_firstRun " << m_firstRun << "max_since " << max_since
<< std::endl;
edm::LogVerbatim("EcalTPGOddWeightIdMapHandler")
<< "m_i_run_number" << m_i_run_number << "m_firstRun " << m_firstRun << "max_since " << max_since;

unsigned int max_run = m_lastRun;
edm::LogInfo("EcalTPGOddWeightIdMapHandler") << "min_run= " << min_run << "max_run= " << max_run;
edm::LogVerbatim("EcalTPGOddWeightIdMapHandler") << "min_run= " << min_run << "max_run= " << max_run;

RunList my_list;
my_list = econn->fetchGlobalRunListByLocation(my_runtag, min_run, max_run, my_locdef);
// my_list=econn->fetchRunListByLocation(my_runtag,min_run,max_run,my_locdef);

std::vector<RunIOV> run_vec = my_list.getRuns();
size_t num_runs = run_vec.size();
edm::LogInfo("EcalTPGOddWeightIdMapHandler") << "number of Mon runs is : " << num_runs;
edm::LogVerbatim("EcalTPGOddWeightIdMapHandler") << "number of Mon runs is : " << num_runs;

unsigned int irun;
if (num_runs > 0) {
for (size_t kr = 0; kr < run_vec.size(); kr++) {
irun = static_cast<unsigned int>(run_vec[kr].getRunNumber());

std::cout << " **************** " << std::endl;
std::cout << " **************** " << std::endl;
std::cout << " run= " << irun << std::endl;
edm::LogVerbatim("EcalTPGOddWeightIdMapHandler") << " **************** ";
edm::LogVerbatim("EcalTPGOddWeightIdMapHandler") << " **************** ";
edm::LogVerbatim("EcalTPGOddWeightIdMapHandler") << " run= " << irun;

// retrieve the data :
std::map<EcalLogicID, RunTPGConfigDat> dataset;
Expand All @@ -145,22 +143,23 @@ void popcon::EcalTPGOddWeightIdMapHandler::getNewObjects() {

// it is all the same for all SM... get the last one

std::cout << " run= " << irun << " tag " << the_config_tag << " version=" << the_config_version << std::endl;
edm::LogVerbatim("EcalTPGOddWeightIdMapHandler")
<< " run= " << irun << " tag " << the_config_tag << " version=" << the_config_version;

// here we should check if it is the same as previous run.

if ((the_config_tag != m_i_tag || the_config_version != m_i_version) && nr > 0) {
std::cout << "the tag is different from last transferred run ... retrieving last config set from DB"
<< std::endl;
edm::LogVerbatim("EcalTPGOddWeightIdMapHandler")
<< "the tag is different from last transferred run ... retrieving last config set from DB";

FEConfigMainInfo fe_main_info;
fe_main_info.setConfigTag(the_config_tag);
fe_main_info.setVersion(the_config_version);

try {
std::cout << " before fetch config set" << std::endl;
edm::LogVerbatim("EcalTPGOddWeightIdMapHandler") << " before fetch config set";
econn->fetchConfigSet(&fe_main_info);
std::cout << " after fetch config set" << std::endl;
edm::LogVerbatim("EcalTPGOddWeightIdMapHandler") << " after fetch config set";

// now get TPGOddWeightIdMap
int weightId = fe_main_info.getWei2Id();
Expand All @@ -171,7 +170,7 @@ void popcon::EcalTPGOddWeightIdMapHandler::getNewObjects() {
econn->fetchConfigSet(&fe_odd_weight_info);
std::map<EcalLogicID, FEConfigOddWeightGroupDat> dataset_TpgWeight;
econn->fetchDataSet(&dataset_TpgWeight, &fe_odd_weight_info);
edm::LogInfo("EcalTPGOddWeightIdMapHandler") << "Got object!";
edm::LogVerbatim("EcalTPGOddWeightIdMapHandler") << "Got object!";
EcalTPGOddWeightIdMap* weightMap = new EcalTPGOddWeightIdMap;
typedef std::map<EcalLogicID, FEConfigOddWeightGroupDat>::const_iterator CIfeweight;
EcalLogicID ecid_xt;
Expand All @@ -194,7 +193,7 @@ void popcon::EcalTPGOddWeightIdMapHandler::getNewObjects() {
++igroups;
}

edm::LogInfo("EcalTPGOddWeightIdMapHandler") << "found " << igroups << "Weight groups";
edm::LogVerbatim("EcalTPGOddWeightIdMapHandler") << "found " << igroups << "Weight groups";

Time_t snc = (Time_t)irun;
m_to_transfer.push_back(std::make_pair((EcalTPGOddWeightIdMap*)weightMap, snc));
Expand All @@ -213,44 +212,44 @@ void popcon::EcalTPGOddWeightIdMapHandler::getNewObjects() {

writeFile("last_tpg_OddweightIdMap_settings.txt");

std::cout
<< " even if the tag/version is not the same, the weightIdMap id is the same -> no transfer needed "
<< std::endl;
edm::LogVerbatim("EcalTPGOddWeightIdMapHandler")
<< " even if the tag/version is not the same, the weightIdMap id is the same -> no transfer needed ";
}

} catch (std::exception& e) {
std::cout << "ERROR: THIS CONFIG DOES NOT EXIST: tag=" << the_config_tag
<< " version=" << the_config_version << std::endl;
std::cout << e.what() << std::endl;
throw cms::Exception("FileReadError") << "ERROR: THIS CONFIG DOES NOT EXIST: tag=" << the_config_tag
<< " version=" << the_config_version << "\n"
<< e.what();
m_i_run_number = irun;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This assignment is now dead code.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point

}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you should not catch the exception, but let the framework do it, please see 7.11 in https://cms-sw.github.io/cms_coding_rules.html

std::cout << " **************** " << std::endl;
edm::LogVerbatim("EcalTPGOddWeightIdMapHandler") << " **************** ";

} else if (nr == 0) {
m_i_run_number = irun;
std::cout << " no tag saved to RUN_TPGCONFIG_DAT by EcalSupervisor -> no transfer needed " << std::endl;
std::cout << " **************** " << std::endl;
edm::LogVerbatim("EcalTPGOddWeightIdMapHandler")
<< " no tag saved to RUN_TPGCONFIG_DAT by EcalSupervisor -> no transfer needed ";
edm::LogVerbatim("EcalTPGOddWeightIdMapHandler") << " **************** ";
} else {
m_i_run_number = irun;
m_i_tag = the_config_tag;
m_i_version = the_config_version;
std::cout << " the tag/version is the same -> no transfer needed " << std::endl;
std::cout << " **************** " << std::endl;
edm::LogVerbatim("EcalTPGOddWeightIdMapHandler") << " the tag/version is the same -> no transfer needed ";
edm::LogVerbatim("EcalTPGOddWeightIdMapHandler") << " **************** ";
writeFile("last_tpg_OddweightIdMap_settings.txt");
}
}
}

delete econn;
} // usual way
edm::LogInfo("EcalTPGOddWeightIdMapHandler") << "Ecal - > end of getNewObjects -----------";
edm::LogVerbatim("EcalTPGOddWeightIdMapHandler") << "Ecal - > end of getNewObjects -----------";
}
void popcon::EcalTPGOddWeightIdMapHandler::readtxtFile() {
std::cout << " reading the input file " << m_file_name << std::endl;
edm::LogVerbatim("EcalTPGOddWeightIdMapHandler") << " reading the input file " << m_file_name;
std::ifstream fInput;
fInput.open(m_file_name);
if (!fInput.is_open()) {
std::cout << "ERROR : cannot open file " << m_file_name << std::endl;
edm::LogVerbatim("EcalTPGOddWeightIdMapHandler") << "ERROR : cannot open file " << m_file_name;
exit(1);
}
unsigned int wloc[5];
Expand All @@ -264,28 +263,27 @@ void popcon::EcalTPGOddWeightIdMapHandler::readtxtFile() {
std::stringstream ss;
ss << line;
ss >> wloc[0] >> wloc[1] >> wloc[2] >> wloc[3] >> wloc[4];
// std::cout << wloc[0] << " " << wloc[1] << " " << wloc[2] << " " << wloc[3] << " " << wloc[4] << std::endl;
w.setValues(wloc[0], wloc[1], wloc[2], wloc[3], wloc[4]);
weightMap->setValue(igroups, w);
igroups++;
}
}
edm::LogInfo("EcalTPGOddWeightIdMapHandler") << "found " << igroups << " Weight groups";
edm::LogVerbatim("EcalTPGOddWeightIdMapHandler") << "found " << igroups << " Weight groups";
try {
Time_t snc = (Time_t)m_firstRun;
m_to_transfer.push_back(std::make_pair((EcalTPGOddWeightIdMap*)weightMap, snc));
} catch (std::exception& e) {
std::cout << "EcalTPGOddWeightIdMapHandler::readtxtFile error : " << e.what() << std::endl;
throw cms::Exception("FileReadError") << "EcalTPGOddWeightIdMapHandler::readtxtFile error : " << e.what();
}
std::cout << " **************** " << std::endl;
edm::LogVerbatim("EcalTPGOddWeightIdMapHandler") << " **************** ";
}

void popcon::EcalTPGOddWeightIdMapHandler::readxmlFile() {
std::cout << " reading the input file " << m_file_name << std::endl;
edm::LogVerbatim("EcalTPGOddWeightIdMapHandler") << " reading the input file " << m_file_name;
std::ifstream fxml;
fxml.open(m_file_name);
if (!fxml.is_open()) {
std::cout << "ERROR : cannot open file " << m_file_name << std::endl;
edm::LogVerbatim("EcalTPGOddWeightIdMapHandler") << "ERROR : cannot open file " << m_file_name;
exit(1);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about replacing this exit() call with throwing an exception? In production code exit() etc are forbidden.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed

}
std::string dummyLine, bid;
Expand All @@ -300,21 +298,19 @@ void popcon::EcalTPGOddWeightIdMapHandler::readxmlFile() {
std::string stt = bid.substr(7, 1);
std::istringstream sc(stt);
sc >> ngroups;
edm::LogInfo("EcalTPGOddWeightIdMapHandler") << "found " << ngroups << " Weight groups";
edm::LogVerbatim("EcalTPGOddWeightIdMapHandler") << "found " << ngroups << " Weight groups";
for (int i = 0; i < 2; i++)
std::getline(fxml, dummyLine); // <item_version>0</item_version>
// std::cout << dummyLine << std::endl;
for (int i = 0; i < ngroups; i++) {
std::getline(fxml, dummyLine); // <item
// std::cout << " group " << i << " first line " << dummyLine << std::endl;
fxml >> bid; // <first
fxml >> bid; // <first
std::size_t found = bid.find("</");
stt = bid.substr(7, found - 7);
std::istringstream sg1(stt);
sg1 >> igroups;
if (igroups != i) {
std::cout << " group " << i << ": " << bid << " igroups " << igroups << std::endl;
exit(-1);
edm::LogVerbatim("EcalTPGOddWeightIdMapHandler") << " group " << i << ": " << bid << " igroups " << igroups;
throw cms::Exception("MismatchError");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about moving the content of LogVerbatim to the exception message?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sounds good

}
for (int i = 0; i < 2; i++)
std::getline(fxml, dummyLine); // < second
Expand All @@ -329,15 +325,14 @@ void popcon::EcalTPGOddWeightIdMapHandler::readxmlFile() {
weightMap->setValue(igroups, w);
for (int i = 0; i < 3; i++)
std::getline(fxml, dummyLine); // </item>
// std::cout << " group " << i << " last line " << dummyLine << std::endl;
}
try {
Time_t snc = (Time_t)m_firstRun;
m_to_transfer.push_back(std::make_pair((EcalTPGOddWeightIdMap*)weightMap, snc));
} catch (std::exception& e) {
std::cout << "EcalTPGOddWeightIdMapHandler::readxmlFile error : " << e.what() << std::endl;
throw cms::Exception("FileReadError") << "EcalTPGOddWeightIdMapHandler::readxmlFile error : " << e.what();
}
Comment on lines 329 to 331
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you should not catch the exception, but let the framework do it, please see 7.11 in https://cms-sw.github.io/cms_coding_rules.html

std::cout << " **************** " << std::endl;
edm::LogVerbatim("EcalTPGOddWeightIdMapHandler") << " **************** ";
}

void popcon::EcalTPGOddWeightIdMapHandler::readFromFile(const char* inputFile) {
Expand Down