-
Notifications
You must be signed in to change notification settings - Fork 4.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Some EcalClusterTools maintainance and energy matrix in Egamma MVA Ntuplizers #25633
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,112 @@ | ||
#ifndef RecoCaloTools_Navigation_CaloRectangle_H | ||
#define RecoCaloTools_Navigation_CaloRectangle_H | ||
|
||
#include "Geometry/CaloTopology/interface/CaloSubdetectorTopology.h" | ||
#include "Geometry/CaloTopology/interface/CaloTopology.h" | ||
|
||
/* | ||
* CaloRectangle is a class to create a rectangular range of DetIds around a | ||
* central DetId. Meant to be used for range-based loops to calculate cluster | ||
* shape variables. | ||
*/ | ||
|
||
struct CaloRectangle { | ||
const int iEtaOrIXMin; | ||
const int iEtaOrIXMax; | ||
const int iPhiOrIYMin; | ||
const int iPhiOrIYMax; | ||
|
||
template<class T> | ||
auto operator()(T home, CaloTopology const& topology); | ||
|
||
}; | ||
|
||
template<class T> | ||
T offsetBy(T start, CaloSubdetectorTopology const& topo, int dIEtaOrIX, int dIPhiOrIY) | ||
{ | ||
for(int i = 0; i < std::abs(dIEtaOrIX) && start != T(0); i++) { | ||
start = dIEtaOrIX > 0 ? topo.goEast(start) : topo.goWest(start); | ||
} | ||
|
||
for(int i = 0; i < std::abs(dIPhiOrIY) && start != T(0); i++) { | ||
start = dIPhiOrIY > 0 ? topo.goNorth(start) : topo.goSouth(start); | ||
} | ||
return start; | ||
} | ||
|
||
template<class T> | ||
class CaloRectangleRange { | ||
|
||
public: | ||
|
||
class Iterator { | ||
|
||
public: | ||
|
||
Iterator(T const& home, int iEtaOrIX, int iPhiOrIY, CaloRectangle const rectangle, CaloSubdetectorTopology const& topology) | ||
: home_(home) | ||
, rectangle_(rectangle) | ||
, topology_(topology) | ||
, iEtaOrIX_(iEtaOrIX) | ||
, iPhiOrIY_(iPhiOrIY) | ||
{} | ||
|
||
Iterator& operator++() { | ||
if(iPhiOrIY_ == rectangle_.iPhiOrIYMax) { | ||
iPhiOrIY_ = rectangle_.iPhiOrIYMin; | ||
iEtaOrIX_++; | ||
} else ++iPhiOrIY_; | ||
return *this; | ||
} | ||
|
||
int iEtaOrIX() const { return iEtaOrIX_; } | ||
int iPhiOrIY() const { return iPhiOrIY_; } | ||
|
||
bool operator==(Iterator const& other) const { return iEtaOrIX_ == other.iEtaOrIX() && iPhiOrIY_ == other.iPhiOrIY(); } | ||
bool operator!=(Iterator const& other) const { return iEtaOrIX_ != other.iEtaOrIX() || iPhiOrIY_ != other.iPhiOrIY(); } | ||
|
||
T operator*() const { return offsetBy(home_, topology_, iEtaOrIX_, iPhiOrIY_); } | ||
|
||
private: | ||
|
||
const T home_; | ||
|
||
const CaloRectangle rectangle_; | ||
CaloSubdetectorTopology const& topology_; | ||
|
||
int iEtaOrIX_; | ||
int iPhiOrIY_; | ||
}; | ||
|
||
public: | ||
CaloRectangleRange(CaloRectangle rectangle, T home, CaloTopology const& topology) | ||
: home_(home) | ||
, rectangle_(rectangle) | ||
, topology_(*topology.getSubdetectorTopology(home)) | ||
{} | ||
|
||
CaloRectangleRange(int size, T home, CaloTopology const& topology) | ||
: home_(home) | ||
, rectangle_{-size, size, -size, size} | ||
, topology_(*topology.getSubdetectorTopology(home)) | ||
{} | ||
|
||
auto begin() { | ||
return Iterator(home_, rectangle_.iEtaOrIXMin, rectangle_.iPhiOrIYMin, rectangle_, topology_); | ||
} | ||
auto end() { | ||
return Iterator(home_, rectangle_.iEtaOrIXMax + 1, rectangle_.iPhiOrIYMin, rectangle_, topology_); | ||
} | ||
|
||
private: | ||
const T home_; | ||
const CaloRectangle rectangle_; | ||
CaloSubdetectorTopology const& topology_; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. One of my long standing issues with EcalClusterTools is that it fairly needlessly uses CaloSubdetectorTopology and we could just hardcode it. I mean iEta 83 will always be next to iEta 84. This could have complications in the phase II for the endcap though we would have to look into this. Anyways one possibility is that we could use this class to hide the CaloTopology from EcalClusterTools (would obv require a bit of a re-write) so it could either pick up a CaloTopology object or a hard coded class. Although maybe better ways to achieve this. Anyways something to think about. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good idea for the future! Yes, that's probably one of the nice things about this class, that these kind of considerations can be brought in here without cluttering the ClusterTools. And what about iEta 0 by the way? That does not exist right? what happens then, will it be jumped over? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh you have limits on it of course. My version (which I use in my private code) knows to skip iEta=0, wrap phi in 1-360 and that 85 is the last crystal. I have versions for HCAL, L1, etc as well. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. eg Incidentally thats the code what was used to develop several of the showershapes so its the "correct version" if there are bugs :) |
||
}; | ||
|
||
template<class T> | ||
auto CaloRectangle::operator()(T home, CaloTopology const& topology) { | ||
return CaloRectangleRange<T>(*this, home, topology); | ||
} | ||
|
||
#endif |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So a usability comment. This is ieta, iphi in the barrel, not ix, iy which is just the endcap.
This might cause user confusion. Obviously there is no good solution here as the variable name changes based on the subdector. In my private code, I've always used iEtaOrIX() / iPhiOrIY() to solve this issue. However I'm more than happy to hear other peoples opinions / suggestions on what is the best course of action here for the names.