From 342bc40426ad8cee611e8ac89db0ed2f6c8fbe1c Mon Sep 17 00:00:00 2001
From: sherwood <peter.sherwood@cern.ch>
Date: Tue, 13 Oct 2020 10:28:05 +0200
Subject: [PATCH 1/4] Optimisation: add check on number of jets passing initial
Conditions to determin if th hypo could be passed.
Add changes to FastReducer
Add: pass th infor colector to JetGroupProduct
---
.../TrigHLTJetHypo/src/FastReducer.cxx | 41 ++++++++++++++-----
.../TrigHLTJetHypo/src/JetGroupProduct.cxx | 18 ++++++--
.../TrigHLTJetHypo/src/JetGroupProduct.h | 5 ++-
3 files changed, 49 insertions(+), 15 deletions(-)
diff --git a/Trigger/TrigHypothesis/TrigHLTJetHypo/src/FastReducer.cxx b/Trigger/TrigHypothesis/TrigHLTJetHypo/src/FastReducer.cxx
index 806abfd022d8..1523604a7e2a 100644
--- a/Trigger/TrigHypothesis/TrigHLTJetHypo/src/FastReducer.cxx
+++ b/Trigger/TrigHypothesis/TrigHLTJetHypo/src/FastReducer.cxx
@@ -147,7 +147,7 @@ bool FastReducer::findInitialJetGroups(const std::vector<int>& leaves,
auto jg = *iter;
if(jg.size() != 1){
- collector->collect("FastReducer", "No jet groups");
+ collector->collect("FastReducer", "Initial jet group size != 1");
return false;
}
@@ -179,10 +179,29 @@ bool FastReducer::findInitialJetGroups(const std::vector<int>& leaves,
}
// check all leaf conditions are satisfied
+ std::set<std::size_t> s_inds;
for (const auto& i : leaves) {
- if ((m_satisfiedBy.at(i)).empty()) { return false; }
+ auto satisfiedBy = m_satisfiedBy.at(i);
+ if (satisfiedBy.empty()) {
+ return false;
+ }
+ for (const auto& j : satisfiedBy){
+ s_inds.insert(j);
+ }
}
-
+
+
+
+ if (m_conditions[0]->capacity() > s_inds.size()) {
+ if (collector){
+ collector -> collect("FastReducer", "too few of jets: " +
+ std::to_string(s_inds.size()) +
+ " condition 0 capacity " +
+ std::to_string(m_conditions[0]->capacity()));
+ }
+ return false;
+ }
+
return true;
}
@@ -218,6 +237,7 @@ bool FastReducer::propagateJetGroups(const Collector& collector){
}
while(!to_process.empty()){
+
auto k = to_process.top();
to_process.pop();
@@ -269,7 +289,7 @@ bool FastReducer::propagate_(std::size_t child,
// Edges are contructed between satisfying jet groups and the parent.
// if any such edge is constructed, the calling rroutine is notified so it
// can scheduling processing the parent as a child.
-
+
std::size_t par = m_tree.parent(child);
// child == 0 do not attempt to process parent of node.
@@ -282,11 +302,12 @@ bool FastReducer::propagate_(std::size_t child,
// eg if condition c1 is satisfied by jg11 and jg12, while its only
// sibling c2 is satisfied by jg21, the external jet groups are
// jg11jg21, jg12jg21. Each of these are flattened.
-
+
+
auto jg_product = JetGroupProduct(siblings, m_satisfiedBy);
-
+
// obtain the next product of hob groups passing siblings
- auto next = jg_product.next();
+ auto next = jg_product.next(collector);
// step through the jet groups found by combining ghe child groups
// check ecach combination to see if it satisfies the parent. If so
@@ -316,7 +337,7 @@ bool FastReducer::propagate_(std::size_t child,
std::set<std::size_t> unique_indices(elem_jgs.begin(),
elem_jgs.end());
if(unique_indices.size() != elem_jgs.size()){
- next = jg_product.next();
+ next = jg_product.next(collector);
continue;
}
@@ -325,7 +346,7 @@ bool FastReducer::propagate_(std::size_t child,
// obtain an index for the new jet group.
auto cur_jg = m_jgIndAllocator(elem_jgs);
if(m_testedBy[par].find(cur_jg) != m_testedBy[par].end()){
- next = jg_product.next();
+ next = jg_product.next(collector);
continue;
}
m_testedBy[par].insert(cur_jg);
@@ -344,7 +365,7 @@ bool FastReducer::propagate_(std::size_t child,
if(collector){recordJetGroup(cur_jg, jg, collector);}
}
- next = jg_product.next();
+ next = jg_product.next(collector);
}
if(collector and !par_satisfied){
diff --git a/Trigger/TrigHypothesis/TrigHLTJetHypo/src/JetGroupProduct.cxx b/Trigger/TrigHypothesis/TrigHLTJetHypo/src/JetGroupProduct.cxx
index e660ab55479b..21ce1a5e7746 100644
--- a/Trigger/TrigHypothesis/TrigHLTJetHypo/src/JetGroupProduct.cxx
+++ b/Trigger/TrigHypothesis/TrigHLTJetHypo/src/JetGroupProduct.cxx
@@ -4,6 +4,7 @@
#include "./JetGroupProduct.h"
#include <set>
+#include <string>
JetGroupProduct::JetGroupProduct(const std::vector<std::size_t>& siblings,
@@ -22,9 +23,18 @@ JetGroupProduct::JetGroupProduct(const std::vector<std::size_t>& siblings,
m_productGen = ProductGen(ends);
}
-std::optional<std::vector<std::size_t>> JetGroupProduct::next(){
+std::optional<std::vector<std::size_t>>
+JetGroupProduct::next(const Collector& collector){
+ unsigned int ipass{0};
+
while(true){
+
+ if(collector){
+ collector->collect("JobGroupProduct::next()",
+ "loop start pass" + std::to_string(ipass++));
+ }
+
auto opt_indices = m_productGen.next();
if(!opt_indices.has_value()){
return std::optional<std::vector<std::size_t>>();
@@ -33,8 +43,8 @@ std::optional<std::vector<std::size_t>> JetGroupProduct::next(){
// indices index job groups in the indJetGroups table
auto indices = *opt_indices;
- // select inicies from the child jet group inicies. Form a vector
- // of inidices.
+ // select indicies from the child jet group indicies. Form a vector
+ // of indices.
std::vector<std::size_t> jg_indices;
for(std::size_t i = 0; i < indices.size(); ++i){
auto s = m_siblings[i];
@@ -44,7 +54,7 @@ std::optional<std::vector<std::size_t>> JetGroupProduct::next(){
}
- // require there are no duplicate inidices - this would be
+ // require there are no duplicate indices - this would be
// rejected by a non-sharing flow network. but remove the
// case early. Sharing is handled otherwise...
std::set<std::size_t> unique_indices(jg_indices.begin(),
diff --git a/Trigger/TrigHypothesis/TrigHLTJetHypo/src/JetGroupProduct.h b/Trigger/TrigHypothesis/TrigHLTJetHypo/src/JetGroupProduct.h
index dc31dc0772dc..0e51a85da75d 100644
--- a/Trigger/TrigHypothesis/TrigHLTJetHypo/src/JetGroupProduct.h
+++ b/Trigger/TrigHypothesis/TrigHLTJetHypo/src/JetGroupProduct.h
@@ -6,12 +6,15 @@
#define TRIGHLTJETHYPO_JETGROUPPRODUCT_H
#include "./ProductGen.h"
+#include "./DebugInfoCollector.h"
#include "TrigHLTJetHypo/TrigHLTJetHypoUtils/HypoJetDefs.h"
#include <vector>
#include <optional>
using CondInd2JetGroupsInds = std::map<int, std::vector<std::size_t>>;
+typedef std::unique_ptr<ITrigJetHypoInfoCollector> Collector;
+
class JetGroupProduct{
/*
* Iterate through the combinations of jet groups.
@@ -29,7 +32,7 @@ class JetGroupProduct{
public:
JetGroupProduct(const std::vector<std::size_t>& siblings,
const CondInd2JetGroupsInds& satisfiedBy);
- std::optional<std::vector<std::size_t>> next();
+ std::optional<std::vector<std::size_t>> next(const Collector&);
private:
const std::vector<std::size_t> m_siblings;
--
GitLab
From 6b96a422e94fcec88d1490b1a7d3395ff5a8d3a7 Mon Sep 17 00:00:00 2001
From: sherwood <peter.sherwood@cern.ch>
Date: Tue, 13 Oct 2020 21:51:36 +0200
Subject: [PATCH 2/4] Avoid a copy by using a reference in
TrigHLTJetHypo/src/FastReducer.cxx
---
Trigger/TrigHypothesis/TrigHLTJetHypo/src/FastReducer.cxx | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Trigger/TrigHypothesis/TrigHLTJetHypo/src/FastReducer.cxx b/Trigger/TrigHypothesis/TrigHLTJetHypo/src/FastReducer.cxx
index 1523604a7e2a..427f083928cf 100644
--- a/Trigger/TrigHypothesis/TrigHLTJetHypo/src/FastReducer.cxx
+++ b/Trigger/TrigHypothesis/TrigHLTJetHypo/src/FastReducer.cxx
@@ -181,7 +181,7 @@ bool FastReducer::findInitialJetGroups(const std::vector<int>& leaves,
// check all leaf conditions are satisfied
std::set<std::size_t> s_inds;
for (const auto& i : leaves) {
- auto satisfiedBy = m_satisfiedBy.at(i);
+ auto& satisfiedBy = m_satisfiedBy.at(i);
if (satisfiedBy.empty()) {
return false;
}
--
GitLab
From a47152bcc2c9f6da268baf219c8aa1f9a14b4f63 Mon Sep 17 00:00:00 2001
From: sherwood <peter.sherwood@cern.ch>
Date: Wed, 14 Oct 2020 23:08:58 +0200
Subject: [PATCH 3/4] Modify viability test in FastReductionMatcher::
findInitialJetGroups()
Original testL the number of initially selected jets was compared to the
number of jets rquird by the root node pof th condition tree. This tst
dos not work for compound trees that include jet sharing
Restrict the test to th important simple case whre the tree consists
of only a root node and leaf children.
---
.../TrigHLTJetHypo/src/FastReducer.cxx | 57 +++++++++++++++----
.../src/FastReductionMatcher.cxx | 2 +-
2 files changed, 48 insertions(+), 11 deletions(-)
diff --git a/Trigger/TrigHypothesis/TrigHLTJetHypo/src/FastReducer.cxx b/Trigger/TrigHypothesis/TrigHLTJetHypo/src/FastReducer.cxx
index 427f083928cf..60e2d6a370cf 100644
--- a/Trigger/TrigHypothesis/TrigHLTJetHypo/src/FastReducer.cxx
+++ b/Trigger/TrigHypothesis/TrigHLTJetHypo/src/FastReducer.cxx
@@ -179,29 +179,66 @@ bool FastReducer::findInitialJetGroups(const std::vector<int>& leaves,
}
// check all leaf conditions are satisfied
- std::set<std::size_t> s_inds;
+ std::map<std::size_t, std::set<size_t>> nChild;
for (const auto& i : leaves) {
auto& satisfiedBy = m_satisfiedBy.at(i);
if (satisfiedBy.empty()) {
return false;
}
for (const auto& j : satisfiedBy){
- s_inds.insert(j);
+ nChild[m_tree.parent(i)].insert(j);
}
}
+ /*
+ For the special but important case where all leaf nodes have
+ the root node as a parent, check that there are enough jets
+ to pass the hypo. This prevents doing a long calculation
+ to discover that the hypo will fail. For example, if the chain
+ requires 10j40, and there are 5 jets that pass the condition,
+ each condition will be satisfied by th 5 jets, and 5^10 combinations
+ will be attempted in th seach for a successful combination. As there
+ are only 5 jets involved, such a combination does not exist.
+
+ This check cannot be applied in the general case. For example,
+ if the root condition requires 4 jets, and has three children,
+ two of which are leaf nodes, while the other is not, then the
+ check will fail the event as no jets have yet ben assigned to the
+ second child, while the full popagation through the tree may pass the
+ event.
+
+ A possible way to tighten the chck would be to forbid children to be
+ separated from thir parent by more than 1 generation.
+ */
+
+ if (nChild.size() == 1) {
+
+ for (const auto& pair : nChild) {
+ const auto& par = pair.first;
+ const auto& children = pair.second;
+
+ if (par != 0) {
+ collector -> collect("FastReducer",
+ "ERROR expect parwnt index to be 0, found " +
+ std::to_string(par));
+ return false;
+ }
- if (m_conditions[0]->capacity() > s_inds.size()) {
- if (collector){
- collector -> collect("FastReducer", "too few of jets: " +
- std::to_string(s_inds.size()) +
- " condition 0 capacity " +
- std::to_string(m_conditions[0]->capacity()));
+ if (m_conditions[par]->capacity() > (children.size())) {
+
+ if (collector){
+ collector -> collect("FastReducer", "too few children. parent ind: " +
+ std::to_string(par) + " with capacity "
+ + std::to_string(m_conditions[par]->capacity()) +
+ " no of children: " +
+ std::to_string(children.size()));
+ }
+ return false;
+ }
}
- return false;
}
-
+
return true;
}
diff --git a/Trigger/TrigHypothesis/TrigHLTJetHypo/src/FastReductionMatcher.cxx b/Trigger/TrigHypothesis/TrigHLTJetHypo/src/FastReductionMatcher.cxx
index f5ca74952ced..7f78b0ca100e 100644
--- a/Trigger/TrigHypothesis/TrigHLTJetHypo/src/FastReductionMatcher.cxx
+++ b/Trigger/TrigHypothesis/TrigHLTJetHypo/src/FastReductionMatcher.cxx
@@ -47,7 +47,7 @@ FastReductionMatcher::match(const HypoJetGroupCIter& groups_b,
m_conditions,
m_tree,
m_sharedNodes,
- jetCollector,
+ jetCollector,
collector);
return std::make_optional<bool>(reducer.pass());
--
GitLab
From 2cef7887bb3215d43dc1071ce77605ff2b2c9a6f Mon Sep 17 00:00:00 2001
From: sherwood <peter.sherwood@cern.ch>
Date: Thu, 15 Oct 2020 16:24:33 +0200
Subject: [PATCH 4/4] Simplefy jet multiplicity test in
TrigHLTJetHypo/FastReducer.cxx
FastReducer.cxx
in method FastReducer::findInitialJetGroups perform jet multiplicity
tests on for tree with tree vector continainoing only 0s
Tree.h
give const iterator access to the parent tree.
---
.../TrigHLTJetHypo/src/FastReducer.cxx | 57 +++++++------------
.../TrigHLTJetHypo/src/Tracer.h | 20 +++++++
.../TrigHypothesis/TrigHLTJetHypo/src/Tree.h | 13 ++++-
3 files changed, 53 insertions(+), 37 deletions(-)
create mode 100644 Trigger/TrigHypothesis/TrigHLTJetHypo/src/Tracer.h
diff --git a/Trigger/TrigHypothesis/TrigHLTJetHypo/src/FastReducer.cxx b/Trigger/TrigHypothesis/TrigHLTJetHypo/src/FastReducer.cxx
index 60e2d6a370cf..49c56a2e1377 100644
--- a/Trigger/TrigHypothesis/TrigHLTJetHypo/src/FastReducer.cxx
+++ b/Trigger/TrigHypothesis/TrigHLTJetHypo/src/FastReducer.cxx
@@ -1,5 +1,5 @@
/*
- Copyright (C) 2002-2019 CERN for the benefit of the ATLAS collaboration
+ Copyright (C) 2002-2020 CERN for the benefit of the ATLAS collaboration
*/
#include "./FastReducer.h"
@@ -141,7 +141,7 @@ bool FastReducer::findInitialJetGroups(const std::vector<int>& leaves,
/*
Will now test the incoming jet groups against the leaf conditions.
*/
-
+
std::size_t ijg{0};
for(auto iter = groups_b; iter != groups_e; ++iter){
auto jg = *iter;
@@ -150,13 +150,14 @@ bool FastReducer::findInitialJetGroups(const std::vector<int>& leaves,
collector->collect("FastReducer", "Initial jet group size != 1");
return false;
}
-
- // if a jet group satisfies a condition, note the fact, and store it by index
+
+ // if a jet group satisfies a condition, note the fact,
+ // and store it by index
bool jg_used{false};
auto cur_jg = m_jgIndAllocator(std::vector<std::size_t>{ijg});
for(const auto& leaf: leaves){
-
+
m_testedBy[leaf].insert(cur_jg);
if (m_conditions[leaf]->isSatisfied(jg, collector)){
jg_used= true;
@@ -177,20 +178,15 @@ bool FastReducer::findInitialJetGroups(const std::vector<int>& leaves,
recordJetGroup(p.first, p.second, collector);
}
}
-
+
// check all leaf conditions are satisfied
- std::map<std::size_t, std::set<size_t>> nChild;
for (const auto& i : leaves) {
auto& satisfiedBy = m_satisfiedBy.at(i);
if (satisfiedBy.empty()) {
return false;
}
- for (const auto& j : satisfiedBy){
- nChild[m_tree.parent(i)].insert(j);
- }
}
-
/*
For the special but important case where all leaf nodes have
the root node as a parent, check that there are enough jets
@@ -201,6 +197,8 @@ bool FastReducer::findInitialJetGroups(const std::vector<int>& leaves,
will be attempted in th seach for a successful combination. As there
are only 5 jets involved, such a combination does not exist.
+ Such trees have a tree vector with all entries == 0.
+
This check cannot be applied in the general case. For example,
if the root condition requires 4 jets, and has three children,
two of which are leaf nodes, while the other is not, then the
@@ -210,39 +208,26 @@ bool FastReducer::findInitialJetGroups(const std::vector<int>& leaves,
A possible way to tighten the chck would be to forbid children to be
separated from thir parent by more than 1 generation.
- */
+ */
- if (nChild.size() == 1) {
-
- for (const auto& pair : nChild) {
- const auto& par = pair.first;
- const auto& children = pair.second;
+ if (std::all_of(m_tree.cbegin(),
+ m_tree.cend(),
+ [](std::size_t i){return i == 0;})) {
+
+ if (m_conditions[0]->capacity() > ijg) {
- if (par != 0) {
- collector -> collect("FastReducer",
- "ERROR expect parwnt index to be 0, found " +
- std::to_string(par));
- return false;
+ if (collector){
+ collector->collect("FastReducer", "too few children. root capacity "
+ + std::to_string(m_conditions[0]->capacity()) +
+ " no of children: " + std::to_string(ijg));
}
- if (m_conditions[par]->capacity() > (children.size())) {
-
- if (collector){
- collector -> collect("FastReducer", "too few children. parent ind: " +
- std::to_string(par) + " with capacity "
- + std::to_string(m_conditions[par]->capacity()) +
- " no of children: " +
- std::to_string(children.size()));
- }
- return false;
- }
+ return false;
}
}
return true;
-}
-
-
+}
bool FastReducer::propagateJetGroups(const Collector& collector){
diff --git a/Trigger/TrigHypothesis/TrigHLTJetHypo/src/Tracer.h b/Trigger/TrigHypothesis/TrigHLTJetHypo/src/Tracer.h
new file mode 100644
index 000000000000..796815b49b31
--- /dev/null
+++ b/Trigger/TrigHypothesis/TrigHLTJetHypo/src/Tracer.h
@@ -0,0 +1,20 @@
+/*
+ Copyright (C) 2002-2020 CERN for the benefit of the ATLAS collaboration
+*/
+
+#ifndef TRIGJETHYPO_TRACER_H
+#define TRIGJETHYPO_TRACER_H
+
+#include <iostream>
+class Tracer {
+public:
+ Tracer(const std::string& msg): m_msg(msg){
+ std::cerr<< "+ " << m_msg << '\n';}
+ ~Tracer() {
+ std::cerr << "- " <<m_msg << '\n';
+ }
+private:
+ std::string m_msg;
+};
+
+#endif
diff --git a/Trigger/TrigHypothesis/TrigHLTJetHypo/src/Tree.h b/Trigger/TrigHypothesis/TrigHLTJetHypo/src/Tree.h
index 06df1b161800..9cc78bfc9bf0 100644
--- a/Trigger/TrigHypothesis/TrigHLTJetHypo/src/Tree.h
+++ b/Trigger/TrigHypothesis/TrigHLTJetHypo/src/Tree.h
@@ -1,5 +1,5 @@
/*
- Copyright (C) 2002-2019 CERN for the benefit of the ATLAS collaboration
+ Copyright (C) 2002-2020 CERN for the benefit of the ATLAS collaboration
*/
#ifndef TRIGHLTJETHYPO_TREE_H
@@ -25,6 +25,17 @@ class Tree{
const std::vector<std::size_t>& leaves() const;
const std::vector<std::size_t>& firstGeneration() const;
+ std::vector<std::size_t>::const_iterator cbegin() const {
+ return m_parents.cbegin();
+ }
+
+
+ std::vector<std::size_t>::const_iterator cend() const {
+ return m_parents.cend();
+ }
+
+
+
std::size_t depth(std::size_t) const;
private:
--
GitLab