From dd265b2e8cd5ab51dce0f8e0794ad76105144219 Mon Sep 17 00:00:00 2001 From: Andrea Bocci Date: Sat, 26 Sep 2015 11:21:13 +0200 Subject: [PATCH 01/12] hltDiff: compare TriggerResults event by event Compare two TriggerResults collections event by event. These can come from two collections in the same file(s), or from two different sets of files. --- HLTrigger/Tools/bin/BuildFile.xml | 16 ++ HLTrigger/Tools/bin/hltDiff.cc | 290 ++++++++++++++++++++++++++++++ 2 files changed, 306 insertions(+) create mode 100644 HLTrigger/Tools/bin/hltDiff.cc diff --git a/HLTrigger/Tools/bin/BuildFile.xml b/HLTrigger/Tools/bin/BuildFile.xml index bcf1790779800..2dee7d1c60fbf 100644 --- a/HLTrigger/Tools/bin/BuildFile.xml +++ b/HLTrigger/Tools/bin/BuildFile.xml @@ -1,3 +1,7 @@ + + + + @@ -7,3 +11,15 @@ + + + + + + + + + + diff --git a/HLTrigger/Tools/bin/hltDiff.cc b/HLTrigger/Tools/bin/hltDiff.cc new file mode 100644 index 0000000000000..0b95876afc44c --- /dev/null +++ b/HLTrigger/Tools/bin/hltDiff.cc @@ -0,0 +1,290 @@ +/* + * usage: hltDiff -o oldfile1.root [oldfile2.root ...] [-O old_label[:old_instance[:old_process]]] + * -n newfile1.root [newfile2.root ...] [-N new_label[:new_instance[:new_process]]] + * -m max_events -v + * + * -o oldfile1.root [oldfile2.root ...] + * input file(s) with the original (reference) trigger results. + * + * -O old_label[:old_instance[:old_process]] + * collection with the original (reference) trigger results; + * the default is "TriggerResults" (without any instance or process name). + * + * -n newfile1.root [newfile2.root ...] + * input file(s) with the trigger results to be compared with the reference; + * to read the trigger results from a different collection in the ame files as + * the reference, use '-n -' and specify the collection with -N (see below). + * + * -N new_label[:new_instance[:new_process]] + * collection with the trigger results to be compared with the reference; + * the default is "TriggerResults" (without any instance or process name). + * + * -m max_events + * compare only the first max_events events; + * the default is to compare all the events in the original (reference) files. + * + * -v + * be verbose: print event-by-event comparison results. + * + * -h + * print this help message, and exit. + */ + +#include +#include +#include +#include +#include + +#include +#include + +#include "FWCore/Common/interface/TriggerNames.h" +#include "FWCore/Utilities/interface/InputTag.h" +#include "DataFormats/Common/interface/TriggerResults.h" +#include "DataFormats/FWLite/interface/Handle.h" +#include "DataFormats/FWLite/interface/Event.h" +#include "DataFormats/FWLite/interface/ChainEvent.h" + +void usage(void) { + std::cout << "\ +usage: hltDiff -o oldfile1.root [oldfile2.root ...] [-O old_label[:old_instance[:old_process]]]\n\ + -n newfile1.root [newfile2.root ...] [-N new_label[:new_instance[:new_process]]] \n\ + -m max_events -v\n\ +\n\ + -o oldfile1.root [oldfile2.root ...]\n\ + input file(s) with the original (reference) trigger results.\n\ +\n\ + -O old_label[:old_instance[:old_process]]\n\ + collection with the original (reference) trigger results;\n\ + the default is 'TriggerResults' (without any instance or process name).\n\ +\n\ + -n newfile1.root [newfile2.root ...]\n\ + input file(s) with the trigger results to be compared with the reference;\n\ + to read the trigger results from a different collection in the ame files as\n\ + the reference, use '-n -' and specify the collection with -N (see below).\n\ +\n\ + -N new_label[:new_instance[:new_process]]\n\ + collection with the trigger results to be compared with the reference;\n\ + the default is 'TriggerResults' (without any instance or process name).\n\ +\n\ + -m max_events\n\ + compare only the first max_events events;\n\ + the default is to compare all the events in the original (reference) files.\n\ +\n\ + -v\n\ + be verbose: print event-by-event comparison results.\n\ +\n\ + -h\n\ + print this help message, and exit." << std::endl; +} + + +const char * decode_path_status(int status) { + static const char * message[] = { "not run", "accepted", "rejected", "error" }; + + if (status > 0 and status < 4) + return message[status]; + else + return "invalid"; +} + +/* +def build_menu(event, results): + tn = event.triggerNames(results) + names = [ tn.triggerName(i) for i in range(results.size()) ] +*/ + +struct TriggerDiff { + TriggerDiff() : count(0), gained(0), lost(0) { } + + unsigned int count; + unsigned int gained; + unsigned int lost; + + static + std::string format(unsigned int value, char sign = '+') { + if (value == 0) + return std::string("-"); + + char buffer[12]; // sign, 10 digits, null + memset(buffer, 0, 12); + + unsigned int digit = 10; + while (value > 0) { + buffer[digit] = value % 10 + 48; + value /= 10; + --digit; + } + buffer[digit] = sign; + + return std::string(buffer + digit); + } +}; + +std::ostream & operator<<(std::ostream & out, TriggerDiff diff) { + out << std::setw(12) << diff.count + << std::setw(12) << TriggerDiff::format(diff.gained, '+') + << std::setw(12) << TriggerDiff::format(diff.lost, '-'); + return out; +} + + + +void compare(std::vector const & old_files, edm::InputTag const & old_label, + std::vector const & new_files, edm::InputTag const & new_label, + unsigned int max_events, bool verbose) { + + std::shared_ptr old_events_p = std::make_shared(old_files); + std::shared_ptr new_events_p; + bool same_files; + + if (new_files.size() == 1 and new_files[0] == "-") { + new_events_p = old_events_p; + same_files = true; + } else { + new_events_p = std::make_shared(new_files); + same_files = false; + } + + fwlite::ChainEvent & old_events = * old_events_p; + fwlite::ChainEvent & new_events = * new_events_p; + + unsigned int counter = 0; + bool new_run = true; + std::vector const * trigger_names = nullptr; + std::vector differences; + + // loop over the reference events + for (old_events.toBegin(); not old_events.atEnd(); ++old_events) { + edm::EventID const& id = old_events.id(); + if (not same_files and not new_events.to(id)) { + std::cerr << "run " << id.run() << ", lumi " << id.luminosityBlock() << ", event " << id.event() << ": not found in the 'new' files, skipping." << std::endl; + continue; + } + + fwlite::Handle old_handle; + old_handle.getByLabel(* old_events.event(), old_label.label().c_str(), old_label.instance().c_str(), old_label.process().c_str()); + auto const & old_results = * old_handle; + + fwlite::Handle new_handle; + new_handle.getByLabel(* new_events.event(), new_label.label().c_str(), new_label.instance().c_str(), new_label.process().c_str()); + auto const & new_results = * new_handle; + + if (new_run) { + new_run = false; + trigger_names = & old_events.triggerNames(old_results).triggerNames(); + if (new_events.triggerNames(new_results).triggerNames() != * trigger_names) { + std::cerr << "Error: inconsistent HLT menus" << std::endl; + return; + } + differences.clear(); + differences.resize(trigger_names->size()); + } + + bool needs_header = true; + for (unsigned int p = 0; p < trigger_names->size(); ++p) { + if (old_results.state(p) == edm::hlt::Pass) + ++differences[p].count; + if (old_results.state(p) == edm::hlt::Pass and new_results.state(p) != edm::hlt::Pass) + ++differences[p].lost; + else if (old_results.state(p) != edm::hlt::Pass and new_results.state(p) == edm::hlt::Pass) + ++differences[p].gained; + + if (verbose) { + if (old_results.state(p) != new_results.state(p) or old_results.index(p) != new_results.index(p)) { + if (needs_header) { + needs_header = false; + std::cout << "run " << id.run() << ", lumi " << id.luminosityBlock() << ", event " << id.event() << ": old result " << old_results.accept() << " , new result " << new_results.accept() << std::endl; + } + std::cout << " Path " << (*trigger_names)[p] << ": " + << " old state is '" << decode_path_status(old_results.state(p)) << "' due to module " << old_results.index(p) + << " new state is '" << decode_path_status(new_results.state(p)) << "' due to module " << new_results.index(p) + << std::endl; + } + } + } + if (not needs_header) + std::cout << std::endl; + + ++counter; + if (max_events and counter >= max_events) + break; + } + + std::cout << std::setw(12) << "Events" << std::setw(12) << "Accepted" << std::setw(12) << "Gained" << std::setw(12) << "Lost" << " " << "Trigger" << std::endl; + for (unsigned int p = 0; p < trigger_names->size(); ++p) + std::cout << std::setw(12) << counter << differences[p] << " " << (*trigger_names)[p] << std::endl; +} + + +int main(int argc, char ** argv) { + // default values + std::vector old_files; + edm::InputTag old_label("TriggerResults"); + std::vector new_files; + edm::InputTag new_label("TriggerResults"); + unsigned int max_events = 0; + bool verbose = false; + + // parse the command line options + int c = -1; + while ((c = getopt(argc, argv, "o:O:n:N:m:vh")) != -1) { + switch (c) { + case 'o': + old_files.push_back(std::string(optarg)); + while (optind < argc) { + if (argv[optind][0] == '-') + break; + old_files.push_back(std::string(argv[optind])); + ++optind; + } + break; + + case 'O': + old_label = edm::InputTag(optarg); + break; + + case 'n': + new_files.push_back(std::string(optarg)); + while (optind < argc) { + if (argv[optind][0] == '-') + break; + new_files.push_back(std::string(argv[optind])); + ++optind; + } + break; + + case 'N': + new_label = edm::InputTag(optarg); + break; + + case 'm': + max_events = atoi(optarg); + break; + + case 'v': + verbose = true; + break; + + case 'h': + usage(); + exit(0); + break; + + default: + std::cerr << "Try 'hltDiff -h' for more information." << std::endl; + exit(1); + break; + } + } + + if (old_files.empty() or new_files.empty()) { + std::cerr << "hltDiff: missing file operand\nTry 'hltDiff -h' for more information." << std::endl; + exit(1); + } + + compare(old_files, old_label, new_files, new_label, max_events, verbose); + + return 0; +} From 44d61c3ba1dd3d22a67c539e022776d4ceb0e11a Mon Sep 17 00:00:00 2001 From: Andrea Bocci Date: Sat, 26 Sep 2015 13:58:21 +0200 Subject: [PATCH 02/12] hltDiff: add GNU-style long options --- HLTrigger/Tools/bin/hltDiff.cc | 102 ++++++++++++++++----------------- 1 file changed, 51 insertions(+), 51 deletions(-) diff --git a/HLTrigger/Tools/bin/hltDiff.cc b/HLTrigger/Tools/bin/hltDiff.cc index 0b95876afc44c..db0eaf64d1147 100644 --- a/HLTrigger/Tools/bin/hltDiff.cc +++ b/HLTrigger/Tools/bin/hltDiff.cc @@ -1,33 +1,7 @@ -/* - * usage: hltDiff -o oldfile1.root [oldfile2.root ...] [-O old_label[:old_instance[:old_process]]] - * -n newfile1.root [newfile2.root ...] [-N new_label[:new_instance[:new_process]]] - * -m max_events -v - * - * -o oldfile1.root [oldfile2.root ...] - * input file(s) with the original (reference) trigger results. - * - * -O old_label[:old_instance[:old_process]] - * collection with the original (reference) trigger results; - * the default is "TriggerResults" (without any instance or process name). - * - * -n newfile1.root [newfile2.root ...] - * input file(s) with the trigger results to be compared with the reference; - * to read the trigger results from a different collection in the ame files as - * the reference, use '-n -' and specify the collection with -N (see below). - * - * -N new_label[:new_instance[:new_process]] - * collection with the trigger results to be compared with the reference; - * the default is "TriggerResults" (without any instance or process name). - * - * -m max_events - * compare only the first max_events events; - * the default is to compare all the events in the original (reference) files. - * - * -v - * be verbose: print event-by-event comparison results. +/* hltDiff: compare TriggerResults event by event * - * -h - * print this help message, and exit. + * Compare two TriggerResults collections event by event. + * These can come from two collections in the same file(s), or from two different sets of files. */ #include @@ -38,6 +12,7 @@ #include #include +#include #include "FWCore/Common/interface/TriggerNames.h" #include "FWCore/Utilities/interface/InputTag.h" @@ -46,39 +21,52 @@ #include "DataFormats/FWLite/interface/Event.h" #include "DataFormats/FWLite/interface/ChainEvent.h" -void usage(void) { - std::cout << "\ -usage: hltDiff -o oldfile1.root [oldfile2.root ...] [-O old_label[:old_instance[:old_process]]]\n\ - -n newfile1.root [newfile2.root ...] [-N new_label[:new_instance[:new_process]]] \n\ - -m max_events -v\n\ +void usage(std::ostream & out) { + out << "\ +usage: hltDiff -o|--old-files FILE1.ROOT [FILE2.ROOT ...] [-O|--old-label LABEL[:INSTANCE[:PROCESS]]]\n\ + -n|--new-files FILE1.ROOT [FILE2.ROOT ...] [-N|--new-label LABEL[:INSTANCE[:PROCESS]]]\n\ + [-m|--max-events MAXEVENTS] [-v|--verbose] [-h|--help]\n\ \n\ - -o oldfile1.root [oldfile2.root ...]\n\ - input file(s) with the original (reference) trigger results.\n\ + -o|--old-files FILE1.ROOT [FILE2.ROOT ...]\n\ + input file(s) with the old (reference) trigger results.\n\ \n\ - -O old_label[:old_instance[:old_process]]\n\ - collection with the original (reference) trigger results;\n\ + -O|--old-label LABEL[:INSTANCE[:PROCESS]]\n\ + collection with the old (reference) trigger results;\n\ the default is 'TriggerResults' (without any instance or process name).\n\ \n\ - -n newfile1.root [newfile2.root ...]\n\ - input file(s) with the trigger results to be compared with the reference;\n\ - to read the trigger results from a different collection in the ame files as\n\ + -n|--new-files FILE1.ROOT [FILE2.ROOT ...]\n\ + input file(s) with the new trigger results to be compared with the reference;\n\ + to read these from a different collection in the same files as\n\ the reference, use '-n -' and specify the collection with -N (see below).\n\ \n\ - -N new_label[:new_instance[:new_process]]\n\ - collection with the trigger results to be compared with the reference;\n\ + -N|--new-label LABEL[:INSTANCE[:PROCESS]]\n\ + collection with the new trigger results to be compared with the reference;\n\ the default is 'TriggerResults' (without any instance or process name).\n\ \n\ - -m max_events\n\ - compare only the first max_events events;\n\ + -m|--max-events MAXEVENTS\n\ + compare only the first MAXEVENTS events;\n\ the default is to compare all the events in the original (reference) files.\n\ \n\ - -v\n\ + -v|--verbose\n\ be verbose: print event-by-event comparison results.\n\ \n\ - -h\n\ + -h|--help\n\ print this help message, and exit." << std::endl; } +void error(std::ostream & out) { + out << "Try 'hltDiff --help' for more information." << std::endl; +} + +void error(std::ostream & out, const char * message) { + out << message << std::endl; + error(out); +} + +void error(std::ostream & out, const std::string & message) { + out << message << std::endl; + error(out); +} const char * decode_path_status(int status) { static const char * message[] = { "not run", "accepted", "rejected", "error" }; @@ -219,6 +207,18 @@ void compare(std::vector const & old_files, edm::InputTag const & o int main(int argc, char ** argv) { + // options + const char optstring[] = "o:O:n:N:m:vh"; + const option longopts[] = { + option{ "old-files", required_argument, nullptr, 'o' }, + option{ "old-label", required_argument, nullptr, 'O' }, + option{ "new-files", required_argument, nullptr, 'n' }, + option{ "new-label", required_argument, nullptr, 'N' }, + option{ "max-events", required_argument, nullptr, 'm' }, + option{ "verbose", no_argument, nullptr, 'v' }, + option{ "help", no_argument, nullptr, 'h' }, + }; + // default values std::vector old_files; edm::InputTag old_label("TriggerResults"); @@ -229,7 +229,7 @@ int main(int argc, char ** argv) { // parse the command line options int c = -1; - while ((c = getopt(argc, argv, "o:O:n:N:m:vh")) != -1) { + while ((c = getopt_long(argc, argv, optstring, longopts, nullptr)) != -1) { switch (c) { case 'o': old_files.push_back(std::string(optarg)); @@ -268,19 +268,19 @@ int main(int argc, char ** argv) { break; case 'h': - usage(); + usage(std::cerr); exit(0); break; default: - std::cerr << "Try 'hltDiff -h' for more information." << std::endl; + error(std::cerr); exit(1); break; } } if (old_files.empty() or new_files.empty()) { - std::cerr << "hltDiff: missing file operand\nTry 'hltDiff -h' for more information." << std::endl; + error(std::cerr, "hltDiff: missing file operand"); exit(1); } From ba107d17ce2dc8c08bf993411ba2c21c991670a4 Mon Sep 17 00:00:00 2001 From: Andrea Bocci Date: Sat, 26 Sep 2015 16:20:15 +0200 Subject: [PATCH 03/12] hltDiff: use HLTConfigData to access the trigger information --- HLTrigger/Tools/bin/hltDiff.cc | 75 +++++++++++++++++++++++++--------- 1 file changed, 56 insertions(+), 19 deletions(-) diff --git a/HLTrigger/Tools/bin/hltDiff.cc b/HLTrigger/Tools/bin/hltDiff.cc index db0eaf64d1147..5cd9b78aa911a 100644 --- a/HLTrigger/Tools/bin/hltDiff.cc +++ b/HLTrigger/Tools/bin/hltDiff.cc @@ -14,12 +14,16 @@ #include #include +#include + #include "FWCore/Common/interface/TriggerNames.h" #include "FWCore/Utilities/interface/InputTag.h" +#include "FWCore/ParameterSet/interface/Registry.h" #include "DataFormats/Common/interface/TriggerResults.h" #include "DataFormats/FWLite/interface/Handle.h" #include "DataFormats/FWLite/interface/Event.h" #include "DataFormats/FWLite/interface/ChainEvent.h" +#include "HLTrigger/HLTcore/interface/HLTConfigData.h" void usage(std::ostream & out) { out << "\ @@ -76,12 +80,32 @@ const char * decode_path_status(int status) { else return "invalid"; } + + +std::string getProcessNameFromBranch(std::string const & branch) { + std::vector> tokens; + boost::split(tokens, branch, boost::is_any_of("_."), boost::token_compress_off); + return boost::copy_range(tokens[3]); +} + +std::unique_ptr getHLTConfigData(fwlite::EventBase const & event, edm::InputTag inputtag) { + auto const & history = event.processHistory(); + auto const & branch = event.getBranchNameFor( edm::Wrapper::typeInfo(), inputtag.label().c_str(), inputtag.instance().c_str(), inputtag.process().c_str() ); + auto const & process = getProcessNameFromBranch( branch ); -/* -def build_menu(event, results): - tn = event.triggerNames(results) - names = [ tn.triggerName(i) for i in range(results.size()) ] -*/ + edm::ProcessConfiguration config; + if (not history.getConfigurationForProcess(process, config)) { + std::cerr << "error: the process " << process << " is not in the Process History" << std::endl; + exit(1); + } + const edm::ParameterSet* pset = edm::pset::Registry::instance()->getMapped(config.parameterSetID()); + if (pset == nullptr) { + std::cerr << "error: the configuration for the process " << process << " is not available in the Provenance" << std::endl; + exit(1); + } + return std::unique_ptr(new HLTConfigData(pset)); +} + struct TriggerDiff { TriggerDiff() : count(0), gained(0), lost(0) { } @@ -138,9 +162,11 @@ void compare(std::vector const & old_files, edm::InputTag const & o fwlite::ChainEvent & old_events = * old_events_p; fwlite::ChainEvent & new_events = * new_events_p; + std::unique_ptr old_config; + std::unique_ptr new_config; + unsigned int counter = 0; bool new_run = true; - std::vector const * trigger_names = nullptr; std::vector differences; // loop over the reference events @@ -161,17 +187,22 @@ void compare(std::vector const & old_files, edm::InputTag const & o if (new_run) { new_run = false; - trigger_names = & old_events.triggerNames(old_results).triggerNames(); - if (new_events.triggerNames(new_results).triggerNames() != * trigger_names) { + old_events.fillParameterSetRegistry(); + new_events.fillParameterSetRegistry(); + + old_config = getHLTConfigData(* old_events.event(), old_label); + new_config = getHLTConfigData(* new_events.event(), new_label); + if (new_config->triggerNames() != old_config->triggerNames()) { std::cerr << "Error: inconsistent HLT menus" << std::endl; - return; + exit(1); } + differences.clear(); - differences.resize(trigger_names->size()); + differences.resize(old_config->size()); } bool needs_header = true; - for (unsigned int p = 0; p < trigger_names->size(); ++p) { + for (unsigned int p = 0; p < old_config->size(); ++p) { if (old_results.state(p) == edm::hlt::Pass) ++differences[p].count; if (old_results.state(p) == edm::hlt::Pass and new_results.state(p) != edm::hlt::Pass) @@ -183,16 +214,22 @@ void compare(std::vector const & old_files, edm::InputTag const & o if (old_results.state(p) != new_results.state(p) or old_results.index(p) != new_results.index(p)) { if (needs_header) { needs_header = false; - std::cout << "run " << id.run() << ", lumi " << id.luminosityBlock() << ", event " << id.event() << ": old result " << old_results.accept() << " , new result " << new_results.accept() << std::endl; + std::cout << "run " << id.run() << ", lumi " << id.luminosityBlock() << ", event " << id.event() << ": old result is '" << old_results.accept() << "', new result is '" << new_results.accept() << "'" << std::endl; } - std::cout << " Path " << (*trigger_names)[p] << ": " - << " old state is '" << decode_path_status(old_results.state(p)) << "' due to module " << old_results.index(p) - << " new state is '" << decode_path_status(new_results.state(p)) << "' due to module " << new_results.index(p) - << std::endl; + std::cout << " Path " << old_config->triggerName(p) << ": "; + if (old_results.state(p) == edm::hlt::Pass) + std::cout << "old state is '" << decode_path_status(old_results.state(p)) << "', "; + else + std::cout << "old state is '" << decode_path_status(old_results.state(p)) << "' due to module '" << old_results.index(p) << "', "; + if (new_results.state(p) == edm::hlt::Pass) + std::cout << "new state is '" << decode_path_status(new_results.state(p)) << "', "; + else + std::cout << "new state is '" << decode_path_status(new_results.state(p)) << "' due to module '" << new_results.index(p) << "', "; + std::cout << std::endl; } } } - if (not needs_header) + if (verbose and not needs_header) std::cout << std::endl; ++counter; @@ -201,8 +238,8 @@ void compare(std::vector const & old_files, edm::InputTag const & o } std::cout << std::setw(12) << "Events" << std::setw(12) << "Accepted" << std::setw(12) << "Gained" << std::setw(12) << "Lost" << " " << "Trigger" << std::endl; - for (unsigned int p = 0; p < trigger_names->size(); ++p) - std::cout << std::setw(12) << counter << differences[p] << " " << (*trigger_names)[p] << std::endl; + for (unsigned int p = 0; p < old_config->size(); ++p) + std::cout << std::setw(12) << counter << differences[p] << " " << old_config->triggerName(p) << std::endl; } From e0fc5550a6f700fecde16cf17e46aa30fc55cc62 Mon Sep 17 00:00:00 2001 From: Andrea Bocci Date: Sat, 26 Sep 2015 23:25:41 +0200 Subject: [PATCH 04/12] hltDiff: add special handling of HLTPrescaler modules --- HLTrigger/Tools/bin/BuildFile.xml | 2 + HLTrigger/Tools/bin/hltDiff.cc | 212 +++++++++++++++++++++--------- 2 files changed, 150 insertions(+), 64 deletions(-) diff --git a/HLTrigger/Tools/bin/BuildFile.xml b/HLTrigger/Tools/bin/BuildFile.xml index 2dee7d1c60fbf..342f6db9bf983 100644 --- a/HLTrigger/Tools/bin/BuildFile.xml +++ b/HLTrigger/Tools/bin/BuildFile.xml @@ -14,11 +14,13 @@ + + diff --git a/HLTrigger/Tools/bin/hltDiff.cc b/HLTrigger/Tools/bin/hltDiff.cc index 5cd9b78aa911a..01af3cec97b03 100644 --- a/HLTrigger/Tools/bin/hltDiff.cc +++ b/HLTrigger/Tools/bin/hltDiff.cc @@ -6,6 +6,7 @@ #include #include +#include #include #include #include @@ -25,11 +26,12 @@ #include "DataFormats/FWLite/interface/ChainEvent.h" #include "HLTrigger/HLTcore/interface/HLTConfigData.h" + void usage(std::ostream & out) { out << "\ usage: hltDiff -o|--old-files FILE1.ROOT [FILE2.ROOT ...] [-O|--old-label LABEL[:INSTANCE[:PROCESS]]]\n\ -n|--new-files FILE1.ROOT [FILE2.ROOT ...] [-N|--new-label LABEL[:INSTANCE[:PROCESS]]]\n\ - [-m|--max-events MAXEVENTS] [-v|--verbose] [-h|--help]\n\ + [-m|--max-events MAXEVENTS] [-p|--prescales] [-v|--verbose] [-h|--help]\n\ \n\ -o|--old-files FILE1.ROOT [FILE2.ROOT ...]\n\ input file(s) with the old (reference) trigger results.\n\ @@ -50,6 +52,9 @@ usage: hltDiff -o|--old-files FILE1.ROOT [FILE2.ROOT ...] [-O|--old-label LABEL[ -m|--max-events MAXEVENTS\n\ compare only the first MAXEVENTS events;\n\ the default is to compare all the events in the original (reference) files.\n\ +\n\ + -p|--prescales\n\ + do not ignore differences caused by HLTPrescaler modules.\n\ \n\ -v|--verbose\n\ be verbose: print event-by-event comparison results.\n\ @@ -72,13 +77,83 @@ void error(std::ostream & out, const std::string & message) { error(out); } -const char * decode_path_status(int status) { - static const char * message[] = { "not run", "accepted", "rejected", "error" }; - if (status > 0 and status < 4) - return message[status]; +class HLTConfigDataEx : public HLTConfigData { +public: + explicit HLTConfigDataEx(HLTConfigData const & data) : + HLTConfigData(data), + moduleTypes_(size()), + prescalers_(size()) + { + for (unsigned int t = 0; t < size(); ++t) { + moduleTypes_[t].resize(size(t)); + prescalers_[t].resize(size(t)); + for (unsigned int m = 0; m < size(t); ++m) { + moduleTypes_[t][m] = data.moduleType(moduleLabel(t, m)); + if (moduleTypes_[t][m] == "HLTPrescaler") + prescalers_[t][m] = true; + } + } + } + + std::vector const & moduleTypes(unsigned int trigger) const { + return moduleTypes_.at(trigger); + } + + + std::string const & moduleType(unsigned int trigger, unsigned int module) const { + return moduleTypes_.at(trigger).at(module); + } + + using HLTConfigData::moduleType; + + bool prescaler(unsigned int trigger, unsigned int module) const { + return prescalers_.at(trigger).at(module); + } + +private: + std::vector> moduleTypes_; + std::vector> prescalers_; +}; + + +enum State { + Ready = edm::hlt::Ready, + Pass = edm::hlt::Pass, + Fail = edm::hlt::Fail, + Exception = edm::hlt::Exception, + Prescaled, + Invalid +}; + +const char * path_state(State state) { + static const char * message[] = { "not run", "accepted", "rejected", "exception", "prescaled", "invalid" }; + + if (state > 0 and state < Invalid) + return message[state]; else - return "invalid"; + return message[Invalid]; +} + +inline +State prescaled_state(int state, int path, int module, HLTConfigDataEx const & config) { + if (state == Fail and config.prescaler(path, module)) + return Prescaled; + return (State) state; +} + +std::string detailed_path_state(State state, int path, int module, HLTConfigDataEx const & config) { + auto const & label = config.moduleLabel(path, module); + auto const & type = config.moduleType(path, module); + + std::stringstream out; + out << "'" << path_state(state) << "'"; + if (state == Fail) + out << " by module '" << module << " " << label << "' [" << type << "]"; + else if (state == Exception) + out << " at module '" << module << " " << label << "' [" << type << "]"; + + return out.str(); } @@ -88,7 +163,7 @@ std::string getProcessNameFromBranch(std::string const & branch) { return boost::copy_range(tokens[3]); } -std::unique_ptr getHLTConfigData(fwlite::EventBase const & event, edm::InputTag inputtag) { +std::unique_ptr getHLTConfigData(fwlite::EventBase const & event, edm::InputTag inputtag) { auto const & history = event.processHistory(); auto const & branch = event.getBranchNameFor( edm::Wrapper::typeInfo(), inputtag.label().c_str(), inputtag.instance().c_str(), inputtag.process().c_str() ); auto const & process = getProcessNameFromBranch( branch ); @@ -103,16 +178,17 @@ std::unique_ptr getHLTConfigData(fwlite::EventBase const & event, std::cerr << "error: the configuration for the process " << process << " is not available in the Provenance" << std::endl; exit(1); } - return std::unique_ptr(new HLTConfigData(pset)); + return std::unique_ptr(new HLTConfigDataEx(HLTConfigData(pset))); } struct TriggerDiff { - TriggerDiff() : count(0), gained(0), lost(0) { } + TriggerDiff() : count(0), gained(0), lost(0), internal(0) { } unsigned int count; unsigned int gained; unsigned int lost; + unsigned int internal; static std::string format(unsigned int value, char sign = '+') { @@ -137,61 +213,54 @@ struct TriggerDiff { std::ostream & operator<<(std::ostream & out, TriggerDiff diff) { out << std::setw(12) << diff.count << std::setw(12) << TriggerDiff::format(diff.gained, '+') - << std::setw(12) << TriggerDiff::format(diff.lost, '-'); + << std::setw(12) << TriggerDiff::format(diff.lost, '-') + << std::setw(12) << TriggerDiff::format(diff.internal, '~'); return out; } - void compare(std::vector const & old_files, edm::InputTag const & old_label, std::vector const & new_files, edm::InputTag const & new_label, - unsigned int max_events, bool verbose) { - - std::shared_ptr old_events_p = std::make_shared(old_files); - std::shared_ptr new_events_p; - bool same_files; - - if (new_files.size() == 1 and new_files[0] == "-") { - new_events_p = old_events_p; - same_files = true; - } else { - new_events_p = std::make_shared(new_files); - same_files = false; - } + unsigned int max_events, bool ignore_prescales, bool verbose) { - fwlite::ChainEvent & old_events = * old_events_p; - fwlite::ChainEvent & new_events = * new_events_p; + std::shared_ptr old_events = std::make_shared(old_files); + std::shared_ptr new_events; + + if (new_files.size() == 1 and new_files[0] == "-") + new_events = old_events; + else + new_events = std::make_shared(new_files); - std::unique_ptr old_config; - std::unique_ptr new_config; + std::unique_ptr old_config; + std::unique_ptr new_config; unsigned int counter = 0; bool new_run = true; std::vector differences; // loop over the reference events - for (old_events.toBegin(); not old_events.atEnd(); ++old_events) { - edm::EventID const& id = old_events.id(); - if (not same_files and not new_events.to(id)) { + for (old_events->toBegin(); not old_events->atEnd(); ++(*old_events)) { + edm::EventID const& id = old_events->id(); + if (new_events != old_events and not new_events->to(id)) { std::cerr << "run " << id.run() << ", lumi " << id.luminosityBlock() << ", event " << id.event() << ": not found in the 'new' files, skipping." << std::endl; continue; } fwlite::Handle old_handle; - old_handle.getByLabel(* old_events.event(), old_label.label().c_str(), old_label.instance().c_str(), old_label.process().c_str()); + old_handle.getByLabel(* old_events->event(), old_label.label().c_str(), old_label.instance().c_str(), old_label.process().c_str()); auto const & old_results = * old_handle; fwlite::Handle new_handle; - new_handle.getByLabel(* new_events.event(), new_label.label().c_str(), new_label.instance().c_str(), new_label.process().c_str()); + new_handle.getByLabel(* new_events->event(), new_label.label().c_str(), new_label.instance().c_str(), new_label.process().c_str()); auto const & new_results = * new_handle; if (new_run) { new_run = false; - old_events.fillParameterSetRegistry(); - new_events.fillParameterSetRegistry(); + old_events->fillParameterSetRegistry(); + new_events->fillParameterSetRegistry(); - old_config = getHLTConfigData(* old_events.event(), old_label); - new_config = getHLTConfigData(* new_events.event(), new_label); + old_config = getHLTConfigData(* old_events->event(), old_label); + new_config = getHLTConfigData(* new_events->event(), new_label); if (new_config->triggerNames() != old_config->triggerNames()) { std::cerr << "Error: inconsistent HLT menus" << std::endl; exit(1); @@ -203,30 +272,35 @@ void compare(std::vector const & old_files, edm::InputTag const & o bool needs_header = true; for (unsigned int p = 0; p < old_config->size(); ++p) { - if (old_results.state(p) == edm::hlt::Pass) + State old_state = prescaled_state(old_results.state(p), p, old_results.index(p), * old_config); + State new_state = prescaled_state(new_results.state(p), p, new_results.index(p), * new_config); + + if (old_state == Pass) ++differences[p].count; - if (old_results.state(p) == edm::hlt::Pass and new_results.state(p) != edm::hlt::Pass) - ++differences[p].lost; - else if (old_results.state(p) != edm::hlt::Pass and new_results.state(p) == edm::hlt::Pass) - ++differences[p].gained; - - if (verbose) { - if (old_results.state(p) != new_results.state(p) or old_results.index(p) != new_results.index(p)) { - if (needs_header) { - needs_header = false; - std::cout << "run " << id.run() << ", lumi " << id.luminosityBlock() << ", event " << id.event() << ": old result is '" << old_results.accept() << "', new result is '" << new_results.accept() << "'" << std::endl; - } - std::cout << " Path " << old_config->triggerName(p) << ": "; - if (old_results.state(p) == edm::hlt::Pass) - std::cout << "old state is '" << decode_path_status(old_results.state(p)) << "', "; - else - std::cout << "old state is '" << decode_path_status(old_results.state(p)) << "' due to module '" << old_results.index(p) << "', "; - if (new_results.state(p) == edm::hlt::Pass) - std::cout << "new state is '" << decode_path_status(new_results.state(p)) << "', "; - else - std::cout << "new state is '" << decode_path_status(new_results.state(p)) << "' due to module '" << new_results.index(p) << "', "; - std::cout << std::endl; + + bool flag = false; + if (not ignore_prescales or (old_state != Prescaled and new_state != Prescaled)) { + if (old_state == Pass and new_state != Pass) { + ++differences[p].lost; + flag = true; + } else if (old_state != Pass and new_state == Pass) { + ++differences[p].gained; + flag = true; + } else if (old_results.index(p) != new_results.index(p)) { + ++differences[p].internal; + flag = true; + } + } + + if (verbose and flag) { + if (needs_header) { + needs_header = false; + std::cout << "run " << id.run() << ", lumi " << id.luminosityBlock() << ", event " << id.event() << ": old result is '" << old_results.accept() << "', new result is '" << new_results.accept() << "'" << std::endl; } + std::cout << " Path " << old_config->triggerName(p) << ": " + << "old state is " << detailed_path_state(old_state, p, old_results.index(p), * old_config) << ", " + << "new state is " << detailed_path_state(new_state, p, new_results.index(p), * new_config) + << std::endl; } } if (verbose and not needs_header) @@ -237,7 +311,7 @@ void compare(std::vector const & old_files, edm::InputTag const & o break; } - std::cout << std::setw(12) << "Events" << std::setw(12) << "Accepted" << std::setw(12) << "Gained" << std::setw(12) << "Lost" << " " << "Trigger" << std::endl; + std::cout << std::setw(12) << "Events" << std::setw(12) << "Accepted" << std::setw(12) << "Gained" << std::setw(12) << "Lost" << std::setw(12) << "Other" << " " << "Trigger" << std::endl; for (unsigned int p = 0; p < old_config->size(); ++p) std::cout << std::setw(12) << counter << differences[p] << " " << old_config->triggerName(p) << std::endl; } @@ -245,13 +319,14 @@ void compare(std::vector const & old_files, edm::InputTag const & o int main(int argc, char ** argv) { // options - const char optstring[] = "o:O:n:N:m:vh"; + const char optstring[] = "o:O:n:N:m:pvh"; const option longopts[] = { option{ "old-files", required_argument, nullptr, 'o' }, option{ "old-label", required_argument, nullptr, 'O' }, option{ "new-files", required_argument, nullptr, 'n' }, option{ "new-label", required_argument, nullptr, 'N' }, option{ "max-events", required_argument, nullptr, 'm' }, + option{ "prescales", no_argument, nullptr, 'p' }, option{ "verbose", no_argument, nullptr, 'v' }, option{ "help", no_argument, nullptr, 'h' }, }; @@ -262,6 +337,7 @@ int main(int argc, char ** argv) { std::vector new_files; edm::InputTag new_label("TriggerResults"); unsigned int max_events = 0; + bool ignore_prescales = true; bool verbose = false; // parse the command line options @@ -300,6 +376,10 @@ int main(int argc, char ** argv) { max_events = atoi(optarg); break; + case 'p': + ignore_prescales = false; + break; + case 'v': verbose = true; break; @@ -316,12 +396,16 @@ int main(int argc, char ** argv) { } } - if (old_files.empty() or new_files.empty()) { - error(std::cerr, "hltDiff: missing file operand"); + if (old_files.empty()) { + error(std::cerr, "hltDiff: please specify the 'old' file(s)"); + exit(1); + } + if (new_files.empty()) { + error(std::cerr, "hltDiff: please specify the 'new' file(s)"); exit(1); } - compare(old_files, old_label, new_files, new_label, max_events, verbose); + compare(old_files, old_label, new_files, new_label, max_events, ignore_prescales, verbose); return 0; } From 4d5acd08c348158b840dc265275eb1650c752d56 Mon Sep 17 00:00:00 2001 From: Andrea Bocci Date: Mon, 28 Sep 2015 02:29:17 +0200 Subject: [PATCH 05/12] hltDiff: save memory by allocating module types in a set, and storing pointers to them in the vectors --- HLTrigger/Tools/bin/hltDiff.cc | 39 ++++++++++++++++++---------------- 1 file changed, 21 insertions(+), 18 deletions(-) diff --git a/HLTrigger/Tools/bin/hltDiff.cc b/HLTrigger/Tools/bin/hltDiff.cc index 01af3cec97b03..c0a58f2c4f1c1 100644 --- a/HLTrigger/Tools/bin/hltDiff.cc +++ b/HLTrigger/Tools/bin/hltDiff.cc @@ -86,23 +86,18 @@ class HLTConfigDataEx : public HLTConfigData { prescalers_(size()) { for (unsigned int t = 0; t < size(); ++t) { - moduleTypes_[t].resize(size(t)); prescalers_[t].resize(size(t)); + moduleTypes_[t].resize(size(t)); for (unsigned int m = 0; m < size(t); ++m) { - moduleTypes_[t][m] = data.moduleType(moduleLabel(t, m)); - if (moduleTypes_[t][m] == "HLTPrescaler") - prescalers_[t][m] = true; + std::string type = data.moduleType(moduleLabel(t, m)); + prescalers_[t][m] = (type == "HLTPrescaler"); + moduleTypes_[t][m] = &* moduleTypeSet_.insert(std::move(type)).first; } } } - std::vector const & moduleTypes(unsigned int trigger) const { - return moduleTypes_.at(trigger); - } - - std::string const & moduleType(unsigned int trigger, unsigned int module) const { - return moduleTypes_.at(trigger).at(module); + return * moduleTypes_.at(trigger).at(module); } using HLTConfigData::moduleType; @@ -112,11 +107,16 @@ class HLTConfigDataEx : public HLTConfigData { } private: - std::vector> moduleTypes_; - std::vector> prescalers_; + std::set moduleTypeSet_; + std::vector> moduleTypes_; + std::vector> prescalers_; }; +const char * event_state(bool state) { + return state ? "accepted" : "rejected"; +} + enum State { Ready = edm::hlt::Ready, Pass = edm::hlt::Pass, @@ -149,9 +149,9 @@ std::string detailed_path_state(State state, int path, int module, HLTConfigData std::stringstream out; out << "'" << path_state(state) << "'"; if (state == Fail) - out << " by module '" << module << " " << label << "' [" << type << "]"; + out << " by module " << module << " '" << label << "' [" << type << "]"; else if (state == Exception) - out << " at module '" << module << " " << label << "' [" << type << "]"; + out << " at module " << module << " '" << label << "' [" << type << "]"; return out.str(); } @@ -295,11 +295,14 @@ void compare(std::vector const & old_files, edm::InputTag const & o if (verbose and flag) { if (needs_header) { needs_header = false; - std::cout << "run " << id.run() << ", lumi " << id.luminosityBlock() << ", event " << id.event() << ": old result is '" << old_results.accept() << "', new result is '" << new_results.accept() << "'" << std::endl; + std::cout << "run " << id.run() << ", lumi " << id.luminosityBlock() << ", event " << id.event() << ": " + << "old result is '" << event_state(old_results.accept()) << "', " + << "new result is '" << event_state(new_results.accept()) << "'" + << std::endl; } - std::cout << " Path " << old_config->triggerName(p) << ": " - << "old state is " << detailed_path_state(old_state, p, old_results.index(p), * old_config) << ", " - << "new state is " << detailed_path_state(new_state, p, new_results.index(p), * new_config) + std::cout << " Path " << old_config->triggerName(p) << ":\n" + << " old state is " << detailed_path_state(old_state, p, old_results.index(p), * old_config) << ",\n" + << " new state is " << detailed_path_state(new_state, p, new_results.index(p), * new_config) << std::endl; } } From ba4ffbf4edbabfec4a8000941962be53db59d78a Mon Sep 17 00:00:00 2001 From: Andrea Bocci Date: Mon, 28 Sep 2015 13:56:58 +0200 Subject: [PATCH 06/12] hltDiff: identify old/new collections by process name --- HLTrigger/Tools/bin/hltDiff.cc | 59 ++++++++++++++++++---------------- 1 file changed, 31 insertions(+), 28 deletions(-) diff --git a/HLTrigger/Tools/bin/hltDiff.cc b/HLTrigger/Tools/bin/hltDiff.cc index c0a58f2c4f1c1..c4294a04514e3 100644 --- a/HLTrigger/Tools/bin/hltDiff.cc +++ b/HLTrigger/Tools/bin/hltDiff.cc @@ -29,25 +29,25 @@ void usage(std::ostream & out) { out << "\ -usage: hltDiff -o|--old-files FILE1.ROOT [FILE2.ROOT ...] [-O|--old-label LABEL[:INSTANCE[:PROCESS]]]\n\ - -n|--new-files FILE1.ROOT [FILE2.ROOT ...] [-N|--new-label LABEL[:INSTANCE[:PROCESS]]]\n\ +usage: hltDiff -o|--old-files FILE1.ROOT [FILE2.ROOT ...] [-O|--old-process LABEL[:INSTANCE[:PROCESS]]]\n\ + -n|--new-files FILE1.ROOT [FILE2.ROOT ...] [-N|--new-process LABEL[:INSTANCE[:PROCESS]]]\n\ [-m|--max-events MAXEVENTS] [-p|--prescales] [-v|--verbose] [-h|--help]\n\ \n\ -o|--old-files FILE1.ROOT [FILE2.ROOT ...]\n\ input file(s) with the old (reference) trigger results.\n\ \n\ - -O|--old-label LABEL[:INSTANCE[:PROCESS]]\n\ - collection with the old (reference) trigger results;\n\ - the default is 'TriggerResults' (without any instance or process name).\n\ + -O|--old-process PROCESS\n\ + process name of the collection with the old (reference) trigger results;\n\ + the default is to take the 'TriggerResults' from the last process.\n\ \n\ -n|--new-files FILE1.ROOT [FILE2.ROOT ...]\n\ input file(s) with the new trigger results to be compared with the reference;\n\ to read these from a different collection in the same files as\n\ the reference, use '-n -' and specify the collection with -N (see below).\n\ \n\ - -N|--new-label LABEL[:INSTANCE[:PROCESS]]\n\ - collection with the new trigger results to be compared with the reference;\n\ - the default is 'TriggerResults' (without any instance or process name).\n\ + -N|--new-process PROCESS\n\ + process name of the collection with the new (reference) trigger results;\n\ + the default is to take the 'TriggerResults' from the last process.\n\ \n\ -m|--max-events MAXEVENTS\n\ compare only the first MAXEVENTS events;\n\ @@ -163,10 +163,13 @@ std::string getProcessNameFromBranch(std::string const & branch) { return boost::copy_range(tokens[3]); } -std::unique_ptr getHLTConfigData(fwlite::EventBase const & event, edm::InputTag inputtag) { +std::unique_ptr getHLTConfigData(fwlite::EventBase const & event, std::string process) { auto const & history = event.processHistory(); - auto const & branch = event.getBranchNameFor( edm::Wrapper::typeInfo(), inputtag.label().c_str(), inputtag.instance().c_str(), inputtag.process().c_str() ); - auto const & process = getProcessNameFromBranch( branch ); + if (process.empty()) { + // determine the process name from the most recent "TriggerResults" object + auto const & branch = event.getBranchNameFor( edm::Wrapper::typeInfo(), "TriggerResults", "", process.c_str() ); + process = getProcessNameFromBranch( branch ); + } edm::ProcessConfiguration config; if (not history.getConfigurationForProcess(process, config)) { @@ -219,8 +222,8 @@ std::ostream & operator<<(std::ostream & out, TriggerDiff diff) { } -void compare(std::vector const & old_files, edm::InputTag const & old_label, - std::vector const & new_files, edm::InputTag const & new_label, +void compare(std::vector const & old_files, std::string const & old_process, + std::vector const & new_files, std::string const & new_process, unsigned int max_events, bool ignore_prescales, bool verbose) { std::shared_ptr old_events = std::make_shared(old_files); @@ -247,11 +250,11 @@ void compare(std::vector const & old_files, edm::InputTag const & o } fwlite::Handle old_handle; - old_handle.getByLabel(* old_events->event(), old_label.label().c_str(), old_label.instance().c_str(), old_label.process().c_str()); + old_handle.getByLabel(* old_events->event(), "TriggerResults", "", old_process.c_str()); auto const & old_results = * old_handle; fwlite::Handle new_handle; - new_handle.getByLabel(* new_events->event(), new_label.label().c_str(), new_label.instance().c_str(), new_label.process().c_str()); + new_handle.getByLabel(* new_events->event(), "TriggerResults", "", new_process.c_str()); auto const & new_results = * new_handle; if (new_run) { @@ -259,8 +262,8 @@ void compare(std::vector const & old_files, edm::InputTag const & o old_events->fillParameterSetRegistry(); new_events->fillParameterSetRegistry(); - old_config = getHLTConfigData(* old_events->event(), old_label); - new_config = getHLTConfigData(* new_events->event(), new_label); + old_config = getHLTConfigData(* old_events->event(), old_process); + new_config = getHLTConfigData(* new_events->event(), new_process); if (new_config->triggerNames() != old_config->triggerNames()) { std::cerr << "Error: inconsistent HLT menus" << std::endl; exit(1); @@ -325,9 +328,9 @@ int main(int argc, char ** argv) { const char optstring[] = "o:O:n:N:m:pvh"; const option longopts[] = { option{ "old-files", required_argument, nullptr, 'o' }, - option{ "old-label", required_argument, nullptr, 'O' }, + option{ "old-process", required_argument, nullptr, 'O' }, option{ "new-files", required_argument, nullptr, 'n' }, - option{ "new-label", required_argument, nullptr, 'N' }, + option{ "new-process", required_argument, nullptr, 'N' }, option{ "max-events", required_argument, nullptr, 'm' }, option{ "prescales", no_argument, nullptr, 'p' }, option{ "verbose", no_argument, nullptr, 'v' }, @@ -336,9 +339,9 @@ int main(int argc, char ** argv) { // default values std::vector old_files; - edm::InputTag old_label("TriggerResults"); + std::string old_process(""); std::vector new_files; - edm::InputTag new_label("TriggerResults"); + std::string new_process(""); unsigned int max_events = 0; bool ignore_prescales = true; bool verbose = false; @@ -348,31 +351,31 @@ int main(int argc, char ** argv) { while ((c = getopt_long(argc, argv, optstring, longopts, nullptr)) != -1) { switch (c) { case 'o': - old_files.push_back(std::string(optarg)); + old_files.emplace_back(optarg); while (optind < argc) { if (argv[optind][0] == '-') break; - old_files.push_back(std::string(argv[optind])); + old_files.emplace_back(argv[optind]); ++optind; } break; case 'O': - old_label = edm::InputTag(optarg); + old_process = optarg; break; case 'n': - new_files.push_back(std::string(optarg)); + new_files.emplace_back(optarg); while (optind < argc) { if (argv[optind][0] == '-') break; - new_files.push_back(std::string(argv[optind])); + new_files.emplace_back(argv[optind]); ++optind; } break; case 'N': - new_label = edm::InputTag(optarg); + new_process = optarg; break; case 'm': @@ -408,7 +411,7 @@ int main(int argc, char ** argv) { exit(1); } - compare(old_files, old_label, new_files, new_label, max_events, ignore_prescales, verbose); + compare(old_files, old_process, new_files, new_process, max_events, ignore_prescales, verbose); return 0; } From 023931dcb4739407b381e1c9a0a760b8c9c32026 Mon Sep 17 00:00:00 2001 From: Andrea Bocci Date: Mon, 28 Sep 2015 17:01:02 +0200 Subject: [PATCH 07/12] hltDiff: add a one-line summary with the number of affected events --- HLTrigger/Tools/bin/hltDiff.cc | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/HLTrigger/Tools/bin/hltDiff.cc b/HLTrigger/Tools/bin/hltDiff.cc index c4294a04514e3..acbfff781546a 100644 --- a/HLTrigger/Tools/bin/hltDiff.cc +++ b/HLTrigger/Tools/bin/hltDiff.cc @@ -238,6 +238,7 @@ void compare(std::vector const & old_files, std::string const & old std::unique_ptr new_config; unsigned int counter = 0; + unsigned int affected = 0; bool new_run = true; std::vector differences; @@ -274,6 +275,7 @@ void compare(std::vector const & old_files, std::string const & old } bool needs_header = true; + bool affected_event = false; for (unsigned int p = 0; p < old_config->size(); ++p) { State old_state = prescaled_state(old_results.state(p), p, old_results.index(p), * old_config); State new_state = prescaled_state(new_results.state(p), p, new_results.index(p), * new_config); @@ -295,6 +297,9 @@ void compare(std::vector const & old_files, std::string const & old } } + if (flag) + affected_event = true; + if (verbose and flag) { if (needs_header) { needs_header = false; @@ -309,6 +314,9 @@ void compare(std::vector const & old_files, std::string const & old << std::endl; } } + if (affected_event) + ++affected; + if (verbose and not needs_header) std::cout << std::endl; @@ -317,6 +325,7 @@ void compare(std::vector const & old_files, std::string const & old break; } + std::cout << "Found " << affected << " events out of " << counter << " with differences:\n" << std::endl; std::cout << std::setw(12) << "Events" << std::setw(12) << "Accepted" << std::setw(12) << "Gained" << std::setw(12) << "Lost" << std::setw(12) << "Other" << " " << "Trigger" << std::endl; for (unsigned int p = 0; p < old_config->size(); ++p) std::cout << std::setw(12) << counter << differences[p] << " " << old_config->triggerName(p) << std::endl; From f12c7758f192829a101eeff0d8eb11321190e362 Mon Sep 17 00:00:00 2001 From: Andrea Bocci Date: Tue, 29 Sep 2015 17:11:33 +0200 Subject: [PATCH 08/12] hltDiff: implement partial comparison for different HLT menus In case the "old" and "new" HLT menus are different, only the common paths are compared. --- HLTrigger/Tools/bin/hltDiff.cc | 237 +++++++++++++++++++++++++++++---- 1 file changed, 211 insertions(+), 26 deletions(-) diff --git a/HLTrigger/Tools/bin/hltDiff.cc b/HLTrigger/Tools/bin/hltDiff.cc index acbfff781546a..3c9c83c10938e 100644 --- a/HLTrigger/Tools/bin/hltDiff.cc +++ b/HLTrigger/Tools/bin/hltDiff.cc @@ -78,35 +78,70 @@ void error(std::ostream & out, const std::string & message) { } -class HLTConfigDataEx : public HLTConfigData { +class HLTConfigInterface { public: - explicit HLTConfigDataEx(HLTConfigData const & data) : - HLTConfigData(data), + virtual unsigned int size() const = 0; + virtual unsigned int size(unsigned int trigger) const = 0; + virtual std::string const & triggerName(unsigned int trigger) const = 0; + virtual unsigned int triggerIndex(unsigned int trigger) const = 0; + virtual std::string const & moduleLabel(unsigned int trigger, unsigned int module) const = 0; + virtual std::string const & moduleType(unsigned int trigger, unsigned int module) const = 0; + virtual bool prescaler(unsigned int trigger, unsigned int module) const = 0; +}; + + +class HLTConfigDataEx : public HLTConfigInterface { +public: + explicit HLTConfigDataEx(HLTConfigData data) : + data_(data), moduleTypes_(size()), prescalers_(size()) { - for (unsigned int t = 0; t < size(); ++t) { + for (unsigned int t = 0; t < data_.size(); ++t) { prescalers_[t].resize(size(t)); moduleTypes_[t].resize(size(t)); - for (unsigned int m = 0; m < size(t); ++m) { - std::string type = data.moduleType(moduleLabel(t, m)); + for (unsigned int m = 0; m < data_.size(t); ++m) { + std::string type = data_.moduleType(moduleLabel(t, m)); prescalers_[t][m] = (type == "HLTPrescaler"); moduleTypes_[t][m] = &* moduleTypeSet_.insert(std::move(type)).first; } } } - std::string const & moduleType(unsigned int trigger, unsigned int module) const { - return * moduleTypes_.at(trigger).at(module); + unsigned int size() const override { + return data_.size(); + } + + unsigned int size(unsigned int trigger) const override { + return data_.size(trigger); + } + + std::vector const & triggerNames() const { + return data_.triggerNames(); } - using HLTConfigData::moduleType; + std::string const & triggerName(unsigned int trigger) const override { + return data_.triggerName(trigger); + } + + unsigned int triggerIndex(unsigned int trigger) const override { + return trigger; + } + + std::string const & moduleLabel(unsigned int trigger, unsigned int module) const override { + return data_.moduleLabel(trigger, module); + } + + std::string const & moduleType(unsigned int trigger, unsigned int module) const override { + return * moduleTypes_.at(trigger).at(module); + } - bool prescaler(unsigned int trigger, unsigned int module) const { + bool prescaler(unsigned int trigger, unsigned int module) const override { return prescalers_.at(trigger).at(module); } private: + HLTConfigData data_; std::set moduleTypeSet_; std::vector> moduleTypes_; std::vector> prescalers_; @@ -117,6 +152,142 @@ const char * event_state(bool state) { return state ? "accepted" : "rejected"; } + +class HLTCommonConfig { +public: + enum class Index { + First = 0, + Second = 1 + }; + + class View : public HLTConfigInterface { + public: + View(HLTCommonConfig const & config, HLTCommonConfig::Index index) : + config_(config), + index_(index) + { } + + virtual unsigned int size() const override; + virtual unsigned int size(unsigned int trigger) const override; + virtual std::string const & triggerName(unsigned int trigger) const override; + virtual unsigned int triggerIndex(unsigned int trigger) const override; + virtual std::string const & moduleLabel(unsigned int trigger, unsigned int module) const override; + virtual std::string const & moduleType(unsigned int trigger, unsigned int module) const override; + virtual bool prescaler(unsigned int trigger, unsigned int module) const override; + + private: + HLTCommonConfig const & config_; + Index index_; + }; + + + HLTCommonConfig(HLTConfigDataEx const & first, HLTConfigDataEx const & second) : + first_(first), + second_(second), + firstView_(*this, Index::First), + secondView_(*this, Index::Second) + { + for (unsigned int f = 0; f < first.size(); ++f) + for (unsigned int s = 0; s < second.size(); ++s) + if (first.triggerName(f) == second.triggerName(s)) { + triggerIndices_.push_back(std::make_pair(f, s)); + break; + } + } + + View const & getView(Index index) const { + if (index == Index::First) + return firstView_; + else + return secondView_; + } + + unsigned int size(Index index) const { + return triggerIndices_.size(); + } + + unsigned int size(Index index, unsigned int trigger) const { + if (index == Index::First) + return first_.size(trigger); + else + return second_.size(trigger); + } + + std::string const & triggerName(Index index, unsigned int trigger) const { + if (index == Index::First) + return first_.triggerName(triggerIndices_.at(trigger).first); + else + return second_.triggerName(triggerIndices_.at(trigger).second); + } + + unsigned int triggerIndex(Index index, unsigned int trigger) const { + if (index == Index::First) + return triggerIndices_.at(trigger).first; + else + return triggerIndices_.at(trigger).second; + } + + std::string const & moduleLabel(Index index, unsigned int trigger, unsigned int module) const { + if (index == Index::First) + return first_.moduleLabel(triggerIndices_.at(trigger).first, module); + else + return second_.moduleLabel(triggerIndices_.at(trigger).second, module); + } + + std::string const & moduleType(Index index, unsigned int trigger, unsigned int module) const { + if (index == Index::First) + return first_.moduleType(triggerIndices_.at(trigger).first, module); + else + return second_.moduleType(triggerIndices_.at(trigger).second, module); + } + + bool prescaler(Index index, unsigned int trigger, unsigned int module) const { + if (index == Index::First) + return first_.prescaler(triggerIndices_.at(trigger).first, module); + else + return second_.prescaler(triggerIndices_.at(trigger).second, module); + } + +private: + HLTConfigDataEx const & first_; + HLTConfigDataEx const & second_; + + View firstView_; + View secondView_; + + std::vector> triggerIndices_; +}; + + +unsigned int HLTCommonConfig::View::size() const { + return config_.size(index_); +} + +unsigned int HLTCommonConfig::View::size(unsigned int trigger) const { + return config_.size(index_, trigger); +} + +std::string const & HLTCommonConfig::View::triggerName(unsigned int trigger) const { + return config_.triggerName(index_, trigger); +} + +unsigned int HLTCommonConfig::View::triggerIndex(unsigned int trigger) const { + return config_.triggerIndex(index_, trigger); +} + +std::string const & HLTCommonConfig::View::moduleLabel(unsigned int trigger, unsigned int module) const { + return config_.moduleLabel(index_, trigger, module); +} + +std::string const & HLTCommonConfig::View::moduleType(unsigned int trigger, unsigned int module) const { + return config_.moduleType(index_, trigger, module); +} + +bool HLTCommonConfig::View::prescaler(unsigned int trigger, unsigned int module) const { + return config_.prescaler(index_, trigger, module); +} + + enum State { Ready = edm::hlt::Ready, Pass = edm::hlt::Pass, @@ -136,13 +307,13 @@ const char * path_state(State state) { } inline -State prescaled_state(int state, int path, int module, HLTConfigDataEx const & config) { +State prescaled_state(int state, int path, int module, HLTConfigInterface const & config) { if (state == Fail and config.prescaler(path, module)) return Prescaled; return (State) state; } -std::string detailed_path_state(State state, int path, int module, HLTConfigDataEx const & config) { +std::string detailed_path_state(State state, int path, int module, HLTConfigInterface const & config) { auto const & label = config.moduleLabel(path, module); auto const & type = config.moduleType(path, module); @@ -170,7 +341,7 @@ std::unique_ptr getHLTConfigData(fwlite::EventBase const & even auto const & branch = event.getBranchNameFor( edm::Wrapper::typeInfo(), "TriggerResults", "", process.c_str() ); process = getProcessNameFromBranch( branch ); } - + edm::ProcessConfiguration config; if (not history.getConfigurationForProcess(process, config)) { std::cerr << "error: the process " << process << " is not in the Process History" << std::endl; @@ -187,7 +358,7 @@ std::unique_ptr getHLTConfigData(fwlite::EventBase const & even struct TriggerDiff { TriggerDiff() : count(0), gained(0), lost(0), internal(0) { } - + unsigned int count; unsigned int gained; unsigned int lost; @@ -234,8 +405,11 @@ void compare(std::vector const & old_files, std::string const & old else new_events = std::make_shared(new_files); - std::unique_ptr old_config; - std::unique_ptr new_config; + std::unique_ptr old_config_data; + std::unique_ptr new_config_data; + std::unique_ptr common_config; + HLTConfigInterface const * old_config = nullptr; + HLTConfigInterface const * new_config = nullptr; unsigned int counter = 0; unsigned int affected = 0; @@ -263,11 +437,19 @@ void compare(std::vector const & old_files, std::string const & old old_events->fillParameterSetRegistry(); new_events->fillParameterSetRegistry(); - old_config = getHLTConfigData(* old_events->event(), old_process); - new_config = getHLTConfigData(* new_events->event(), new_process); - if (new_config->triggerNames() != old_config->triggerNames()) { - std::cerr << "Error: inconsistent HLT menus" << std::endl; - exit(1); + old_config_data = getHLTConfigData(* old_events->event(), old_process); + new_config_data = getHLTConfigData(* new_events->event(), new_process); + if (new_config_data->triggerNames() == old_config_data->triggerNames()) { + old_config = old_config_data.get(); + new_config = new_config_data.get(); + } else { + common_config = std::unique_ptr(new HLTCommonConfig(*old_config_data, *new_config_data)); + old_config = & common_config->getView(HLTCommonConfig::Index::First); + new_config = & common_config->getView(HLTCommonConfig::Index::Second); + std::cerr << "Warning: old and new TriggerResults come from different HLT menus. Only the common triggers will be compared:" << std::endl; + for (unsigned int i = 0; i < old_config->size(); ++i) + std::cerr << " " << old_config->triggerName(i) << std::endl; + std::cerr << std::endl; } differences.clear(); @@ -277,8 +459,11 @@ void compare(std::vector const & old_files, std::string const & old bool needs_header = true; bool affected_event = false; for (unsigned int p = 0; p < old_config->size(); ++p) { - State old_state = prescaled_state(old_results.state(p), p, old_results.index(p), * old_config); - State new_state = prescaled_state(new_results.state(p), p, new_results.index(p), * new_config); + // FIXME explicitly converting the indices is a hack, it should be properly encapsulated instead + unsigned int old_index = old_config->triggerIndex(p); + unsigned int new_index = new_config->triggerIndex(p); + State old_state = prescaled_state(old_results.state(old_index), p, old_results.index(old_index), * old_config); + State new_state = prescaled_state(new_results.state(new_index), p, new_results.index(new_index), * new_config); if (old_state == Pass) ++differences[p].count; @@ -291,7 +476,7 @@ void compare(std::vector const & old_files, std::string const & old } else if (old_state != Pass and new_state == Pass) { ++differences[p].gained; flag = true; - } else if (old_results.index(p) != new_results.index(p)) { + } else if (old_results.index(old_index) != new_results.index(new_index)) { ++differences[p].internal; flag = true; } @@ -309,8 +494,8 @@ void compare(std::vector const & old_files, std::string const & old << std::endl; } std::cout << " Path " << old_config->triggerName(p) << ":\n" - << " old state is " << detailed_path_state(old_state, p, old_results.index(p), * old_config) << ",\n" - << " new state is " << detailed_path_state(new_state, p, new_results.index(p), * new_config) + << " old state is " << detailed_path_state(old_state, p, old_results.index(old_index), * old_config) << ",\n" + << " new state is " << detailed_path_state(new_state, p, new_results.index(new_index), * new_config) << std::endl; } } From 6bcf0801e72621655febdf4f97ef3158f6747324 Mon Sep 17 00:00:00 2001 From: Andrea Bocci Date: Tue, 29 Sep 2015 22:48:43 +0200 Subject: [PATCH 09/12] hltDiff: print trigger candidates for filters with a different decision --- HLTrigger/Tools/bin/hltDiff.cc | 132 +++++++++++++++++++++++++-------- 1 file changed, 101 insertions(+), 31 deletions(-) diff --git a/HLTrigger/Tools/bin/hltDiff.cc b/HLTrigger/Tools/bin/hltDiff.cc index 3c9c83c10938e..e47ed4fb46cc8 100644 --- a/HLTrigger/Tools/bin/hltDiff.cc +++ b/HLTrigger/Tools/bin/hltDiff.cc @@ -21,6 +21,8 @@ #include "FWCore/Utilities/interface/InputTag.h" #include "FWCore/ParameterSet/interface/Registry.h" #include "DataFormats/Common/interface/TriggerResults.h" +#include "DataFormats/HLTReco/interface/TriggerObject.h" +#include "DataFormats/HLTReco/interface/TriggerEvent.h" #include "DataFormats/FWLite/interface/Handle.h" #include "DataFormats/FWLite/interface/Event.h" #include "DataFormats/FWLite/interface/ChainEvent.h" @@ -57,7 +59,8 @@ usage: hltDiff -o|--old-files FILE1.ROOT [FILE2.ROOT ...] [-O|--old-process LABE do not ignore differences caused by HLTPrescaler modules.\n\ \n\ -v|--verbose\n\ - be verbose: print event-by-event comparison results.\n\ + be verbose: print event-by-event comparison results;\n\ + use this option twice to print the trigger candidates of the affected filters.\n\ \n\ -h|--help\n\ print this help message, and exit." << std::endl; @@ -80,6 +83,7 @@ void error(std::ostream & out, const std::string & message) { class HLTConfigInterface { public: + virtual std::string const & processName() const = 0; virtual unsigned int size() const = 0; virtual unsigned int size(unsigned int trigger) const = 0; virtual std::string const & triggerName(unsigned int trigger) const = 0; @@ -108,35 +112,39 @@ class HLTConfigDataEx : public HLTConfigInterface { } } - unsigned int size() const override { + virtual std::string const & processName() const override { + return data_.processName(); + } + + virtual unsigned int size() const override { return data_.size(); } - unsigned int size(unsigned int trigger) const override { + virtual unsigned int size(unsigned int trigger) const override { return data_.size(trigger); } - std::vector const & triggerNames() const { + virtual std::vector const & triggerNames() const { return data_.triggerNames(); } - std::string const & triggerName(unsigned int trigger) const override { + virtual std::string const & triggerName(unsigned int trigger) const override { return data_.triggerName(trigger); } - unsigned int triggerIndex(unsigned int trigger) const override { + virtual unsigned int triggerIndex(unsigned int trigger) const override { return trigger; } - std::string const & moduleLabel(unsigned int trigger, unsigned int module) const override { + virtual std::string const & moduleLabel(unsigned int trigger, unsigned int module) const override { return data_.moduleLabel(trigger, module); } - std::string const & moduleType(unsigned int trigger, unsigned int module) const override { + virtual std::string const & moduleType(unsigned int trigger, unsigned int module) const override { return * moduleTypes_.at(trigger).at(module); } - bool prescaler(unsigned int trigger, unsigned int module) const override { + virtual bool prescaler(unsigned int trigger, unsigned int module) const override { return prescalers_.at(trigger).at(module); } @@ -167,6 +175,7 @@ class HLTCommonConfig { index_(index) { } + virtual std::string const & processName() const override; virtual unsigned int size() const override; virtual unsigned int size(unsigned int trigger) const override; virtual std::string const & triggerName(unsigned int trigger) const override; @@ -202,6 +211,13 @@ class HLTCommonConfig { return secondView_; } + std::string const & processName(Index index) const { + if (index == Index::First) + return first_.processName(); + else + return second_.processName(); + } + unsigned int size(Index index) const { return triggerIndices_.size(); } @@ -259,6 +275,10 @@ class HLTCommonConfig { }; +std::string const & HLTCommonConfig::View::processName() const { + return config_.processName(index_); +} + unsigned int HLTCommonConfig::View::size() const { return config_.size(index_); } @@ -313,18 +333,45 @@ State prescaled_state(int state, int path, int module, HLTConfigInterface const return (State) state; } -std::string detailed_path_state(State state, int path, int module, HLTConfigInterface const & config) { +void print_detailed_path_state(std::ostream & out, State state, int path, int module, HLTConfigInterface const & config) { auto const & label = config.moduleLabel(path, module); auto const & type = config.moduleType(path, module); - std::stringstream out; out << "'" << path_state(state) << "'"; if (state == Fail) out << " by module " << module << " '" << label << "' [" << type << "]"; else if (state == Exception) out << " at module " << module << " '" << label << "' [" << type << "]"; +} - return out.str(); +void print_trigger_candidates(std::ostream & out, trigger::TriggerEvent const & summary, edm::InputTag const & filter) { + // find the index of the collection of trigger candidates corresponding to the filter + unsigned int index = summary.filterIndex(filter); + + if (index >= summary.sizeFilters()) { + // the collection of trigger candidates corresponding to the filter could not be found + out << " not found\n"; + return; + } + + if (summary.filterKeys(index).empty()) { + // the collection of trigger candidates corresponding to the filter is empty + out << " none\n"; + return; + } + + for (unsigned int i = 0; i < summary.filterKeys(index).size(); ++i) { + auto key = summary.filterKeys(index)[i]; + auto id = summary.filterIds(index)[i]; + trigger::TriggerObject const & candidate = summary.getObjects().at(key); + out << " " + << "filter id: " << id << ", " + << "object id: " << candidate.id() << ", " + << "pT: " << candidate.pt() << ", " + << "eta: " << candidate.eta() << ", " + << "phi: " << candidate.phi() << ", " + << "mass: " << candidate.mass() << "\n"; + } } @@ -395,7 +442,7 @@ std::ostream & operator<<(std::ostream & out, TriggerDiff diff) { void compare(std::vector const & old_files, std::string const & old_process, std::vector const & new_files, std::string const & new_process, - unsigned int max_events, bool ignore_prescales, bool verbose) { + unsigned int max_events, bool ignore_prescales, int verbose) { std::shared_ptr old_events = std::make_shared(old_files); std::shared_ptr new_events; @@ -424,14 +471,22 @@ void compare(std::vector const & old_files, std::string const & old continue; } - fwlite::Handle old_handle; - old_handle.getByLabel(* old_events->event(), "TriggerResults", "", old_process.c_str()); - auto const & old_results = * old_handle; + fwlite::Handle old_results_h; + old_results_h.getByLabel(* old_events->event(), "TriggerResults", "", old_process.c_str()); + auto const & old_results = * old_results_h; + + fwlite::Handle old_summary_h; + old_summary_h.getByLabel(* old_events->event(), "hltTriggerSummaryAOD", "", old_process.c_str()); + auto const & old_summary = * old_summary_h; fwlite::Handle new_handle; new_handle.getByLabel(* new_events->event(), "TriggerResults", "", new_process.c_str()); auto const & new_results = * new_handle; + fwlite::Handle new_summary_h; + new_summary_h.getByLabel(* new_events->event(), "hltTriggerSummaryAOD", "", new_process.c_str()); + auto const & new_summary = * new_summary_h; + if (new_run) { new_run = false; old_events->fillParameterSetRegistry(); @@ -482,27 +537,42 @@ void compare(std::vector const & old_files, std::string const & old } } - if (flag) + if (flag) { affected_event = true; - if (verbose and flag) { - if (needs_header) { - needs_header = false; - std::cout << "run " << id.run() << ", lumi " << id.luminosityBlock() << ", event " << id.event() << ": " - << "old result is '" << event_state(old_results.accept()) << "', " - << "new result is '" << event_state(new_results.accept()) << "'" - << std::endl; + if (verbose > 0) { + if (needs_header) { + needs_header = false; + std::cout << "run " << id.run() << ", lumi " << id.luminosityBlock() << ", event " << id.event() << ": " + << "old result is '" << event_state(old_results.accept()) << "', " + << "new result is '" << event_state(new_results.accept()) << "'" + << std::endl; + } + // print the Trigger path and filter responsible for the discrepancy + std::cout << " Path " << old_config->triggerName(p) << ":\n" + << " old state is "; + print_detailed_path_state(std::cout, old_state, p, old_results.index(old_index), * old_config); + std::cout << ",\n" + << " new state is "; + print_detailed_path_state(std::cout, new_state, p, new_results.index(new_index), * new_config); + std::cout << std::endl; + } + if (verbose > 1) { + // print TriggerObjects for the filter responsible for the discrepancy + unsigned int module = std::min(old_results.index(old_index), new_results.index(new_index)); + std::cout << " Filter " << old_config->moduleLabel(p, module) << ":\n"; + std::cout << " old trigger candidates:\n"; + print_trigger_candidates(std::cout, old_summary, edm::InputTag(old_config->moduleLabel(p, module), "", old_config->processName())); + std::cout << " new trigger candidates:\n"; + print_trigger_candidates(std::cout, new_summary, edm::InputTag(new_config->moduleLabel(p, module), "", new_config->processName())); + std::cout << std::endl; } - std::cout << " Path " << old_config->triggerName(p) << ":\n" - << " old state is " << detailed_path_state(old_state, p, old_results.index(old_index), * old_config) << ",\n" - << " new state is " << detailed_path_state(new_state, p, new_results.index(new_index), * new_config) - << std::endl; } } if (affected_event) ++affected; - if (verbose and not needs_header) + if (verbose > 0 and not needs_header) std::cout << std::endl; ++counter; @@ -538,7 +608,7 @@ int main(int argc, char ** argv) { std::string new_process(""); unsigned int max_events = 0; bool ignore_prescales = true; - bool verbose = false; + unsigned int verbose = 0; // parse the command line options int c = -1; @@ -581,7 +651,7 @@ int main(int argc, char ** argv) { break; case 'v': - verbose = true; + ++verbose; break; case 'h': From 9358824478ede527e17fa5b183c407b8058b18f6 Mon Sep 17 00:00:00 2001 From: Andrea Bocci Date: Wed, 30 Sep 2015 20:17:11 +0200 Subject: [PATCH 10/12] hltDiff: print trigger candidates for events with a different decision --- HLTrigger/Tools/bin/hltDiff.cc | 59 +++++++++++++++++++++++++++++++--- 1 file changed, 54 insertions(+), 5 deletions(-) diff --git a/HLTrigger/Tools/bin/hltDiff.cc b/HLTrigger/Tools/bin/hltDiff.cc index e47ed4fb46cc8..c3b39373e2f71 100644 --- a/HLTrigger/Tools/bin/hltDiff.cc +++ b/HLTrigger/Tools/bin/hltDiff.cc @@ -59,8 +59,10 @@ usage: hltDiff -o|--old-files FILE1.ROOT [FILE2.ROOT ...] [-O|--old-process LABE do not ignore differences caused by HLTPrescaler modules.\n\ \n\ -v|--verbose\n\ - be verbose: print event-by-event comparison results;\n\ - use this option twice to print the trigger candidates of the affected filters.\n\ + be (more) verbose:\n\ + use once to print event-by-event comparison results;\n\ + use twice to print the trigger candidates of the affected filters;\n\ + use three times to print all the trigger candidates for the affected events.\n\ \n\ -h|--help\n\ print this help message, and exit." << std::endl; @@ -374,6 +376,35 @@ void print_trigger_candidates(std::ostream & out, trigger::TriggerEvent const & } } +void print_trigger_collection(std::ostream & out, trigger::TriggerEvent const & summary, std::string const & tag) { + auto iterator = std::find(summary.collectionTags().begin(), summary.collectionTags().end(), tag); + if (iterator == summary.collectionTags().end()) { + // the collection of trigger candidates could not be found + out << " not found\n"; + return; + } + + unsigned int index = iterator - summary.collectionTags().begin(); + unsigned int begin = (index == 0) ? 0 : summary.collectionKey(index - 1); + unsigned int end = summary.collectionKey(index); + + if (end == begin) { + // the collection of trigger candidates is empty + out << " none\n"; + return; + } + + for (unsigned int key = begin; key < end; ++key) { + trigger::TriggerObject const & candidate = summary.getObjects().at(key); + out << " " + << "object id: " << candidate.id() << ", " + << "pT: " << candidate.pt() << ", " + << "eta: " << candidate.eta() << ", " + << "phi: " << candidate.phi() << ", " + << "mass: " << candidate.mass() << "\n"; + } +} + std::string getProcessNameFromBranch(std::string const & branch) { std::vector> tokens; @@ -465,12 +496,15 @@ void compare(std::vector const & old_files, std::string const & old // loop over the reference events for (old_events->toBegin(); not old_events->atEnd(); ++(*old_events)) { + + // seek the same event in the "new" files edm::EventID const& id = old_events->id(); if (new_events != old_events and not new_events->to(id)) { std::cerr << "run " << id.run() << ", lumi " << id.luminosityBlock() << ", event " << id.event() << ": not found in the 'new' files, skipping." << std::endl; continue; } + // read the TriggerResults and TriggerEvent fwlite::Handle old_results_h; old_results_h.getByLabel(* old_events->event(), "TriggerResults", "", old_process.c_str()); auto const & old_results = * old_results_h; @@ -487,6 +521,7 @@ void compare(std::vector const & old_files, std::string const & old new_summary_h.getByLabel(* new_events->event(), "hltTriggerSummaryAOD", "", new_process.c_str()); auto const & new_summary = * new_summary_h; + // initialise the trigger configuration if (new_run) { new_run = false; old_events->fillParameterSetRegistry(); @@ -511,6 +546,7 @@ void compare(std::vector const & old_files, std::string const & old differences.resize(old_config->size()); } + // compare the TriggerResults bool needs_header = true; bool affected_event = false; for (unsigned int p = 0; p < old_config->size(); ++p) { @@ -565,15 +601,28 @@ void compare(std::vector const & old_files, std::string const & old print_trigger_candidates(std::cout, old_summary, edm::InputTag(old_config->moduleLabel(p, module), "", old_config->processName())); std::cout << " new trigger candidates:\n"; print_trigger_candidates(std::cout, new_summary, edm::InputTag(new_config->moduleLabel(p, module), "", new_config->processName())); - std::cout << std::endl; } + if (verbose > 0) + std::cout << std::endl; } } if (affected_event) ++affected; - if (verbose > 0 and not needs_header) - std::cout << std::endl; + // compare the TriggerEvent + if (affected_event and verbose > 2) { + std::set names; + names.insert(old_summary.collectionTags().begin(), old_summary.collectionTags().end()); + names.insert(new_summary.collectionTags().begin(), new_summary.collectionTags().end()); + for (auto const & collection: names) { + std::cout << " Collection " << collection << ":\n"; + std::cout << " old trigger candidates:\n"; + print_trigger_collection(std::cout, old_summary, collection); + std::cout << " new trigger candidates:\n"; + print_trigger_collection(std::cout, new_summary, collection); + std::cout << std::endl; + } + } ++counter; if (max_events and counter >= max_events) From 9bfc08bcce6000931e9c6497999b65bcf256156d Mon Sep 17 00:00:00 2001 From: Andrea Bocci Date: Mon, 5 Oct 2015 18:53:10 +0200 Subject: [PATCH 11/12] hltDiff: add protection for non-existing input files --- HLTrigger/Tools/bin/BuildFile.xml | 1 + HLTrigger/Tools/bin/hltDiff.cc | 43 +++++++++++++++++++++++++++++-- 2 files changed, 42 insertions(+), 2 deletions(-) diff --git a/HLTrigger/Tools/bin/BuildFile.xml b/HLTrigger/Tools/bin/BuildFile.xml index 342f6db9bf983..9fd426691f8d6 100644 --- a/HLTrigger/Tools/bin/BuildFile.xml +++ b/HLTrigger/Tools/bin/BuildFile.xml @@ -15,6 +15,7 @@ + diff --git a/HLTrigger/Tools/bin/hltDiff.cc b/HLTrigger/Tools/bin/hltDiff.cc index c3b39373e2f71..fb6a4e2a42a95 100644 --- a/HLTrigger/Tools/bin/hltDiff.cc +++ b/HLTrigger/Tools/bin/hltDiff.cc @@ -16,6 +16,7 @@ #include #include +#include #include "FWCore/Common/interface/TriggerNames.h" #include "FWCore/Utilities/interface/InputTag.h" @@ -471,17 +472,55 @@ std::ostream & operator<<(std::ostream & out, TriggerDiff diff) { } +bool check_file(std::string const & file) { + boost::filesystem::path p(file); + + // check if the file exists + if (not boost::filesystem::exists(p)) + return false; + + // resolve the file name to canonical form + p = boost::filesystem::canonical(p); + if (not boost::filesystem::exists(p)) + return false; + + // check for a regular file + if (not boost::filesystem::is_regular_file(p)) + return false; + + return true; +} + + +bool check_files(std::vector const & files) { + bool flag = true; + for (auto const & file: files) + if (not check_file(file)) { + flag = false; + std::cerr << "hltDiff: error: file " << file << " does not exist, or is not a regular file." << std::endl; + } + return flag; +} + + void compare(std::vector const & old_files, std::string const & old_process, std::vector const & new_files, std::string const & new_process, unsigned int max_events, bool ignore_prescales, int verbose) { - std::shared_ptr old_events = std::make_shared(old_files); + std::shared_ptr old_events; std::shared_ptr new_events; + if (check_files(old_files)) + old_events = std::make_shared(old_files); + else + return; + if (new_files.size() == 1 and new_files[0] == "-") new_events = old_events; - else + else if (check_files(new_files)) new_events = std::make_shared(new_files); + else + return; std::unique_ptr old_config_data; std::unique_ptr new_config_data; From f39a106501e615fd18fe3e61edb25b37bbc7b01f Mon Sep 17 00:00:00 2001 From: Andrea Bocci Date: Mon, 5 Oct 2015 19:10:48 +0200 Subject: [PATCH 12/12] hltDiff: add protection for empty input files --- HLTrigger/Tools/bin/hltDiff.cc | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/HLTrigger/Tools/bin/hltDiff.cc b/HLTrigger/Tools/bin/hltDiff.cc index fb6a4e2a42a95..f6ec50efeaf54 100644 --- a/HLTrigger/Tools/bin/hltDiff.cc +++ b/HLTrigger/Tools/bin/hltDiff.cc @@ -668,10 +668,14 @@ void compare(std::vector const & old_files, std::string const & old break; } - std::cout << "Found " << affected << " events out of " << counter << " with differences:\n" << std::endl; - std::cout << std::setw(12) << "Events" << std::setw(12) << "Accepted" << std::setw(12) << "Gained" << std::setw(12) << "Lost" << std::setw(12) << "Other" << " " << "Trigger" << std::endl; - for (unsigned int p = 0; p < old_config->size(); ++p) - std::cout << std::setw(12) << counter << differences[p] << " " << old_config->triggerName(p) << std::endl; + if (not counter) { + std::cout << "There are no common events between the old and new files." << std::endl; + } else { + std::cout << "Found " << affected << " events out of " << counter << " with differences:\n" << std::endl; + std::cout << std::setw(12) << "Events" << std::setw(12) << "Accepted" << std::setw(12) << "Gained" << std::setw(12) << "Lost" << std::setw(12) << "Other" << " " << "Trigger" << std::endl; + for (unsigned int p = 0; p < old_config->size(); ++p) + std::cout << std::setw(12) << counter << differences[p] << " " << old_config->triggerName(p) << std::endl; + } }