convenience utilities for configuring fftjet jet corrections #3810
Conversation
|
A new Pull Request was created by @igv4321 for CMSSW_7_1_X. convenience utilities for configuring fftjet jet corrections It involves the following packages: JetMETCorrections/FFTJetModules @nclopezo, @vadler, @cmsbuild, @Degano, @monttj can you please review it and eventually sign? Thanks. |
| double* buf, const unsigned dim) const | ||
| { | ||
| if (dim != 1) | ||
| throw cms::Exception("FFTJetBadConfig") |
There was a problem hiding this comment.
Please do not throw at run time. An edm::LogError is preferred.
Where is dim determined? Is it possibly known at c'tor time already?
Beside that, it is not used in the function. So, why should the function break due to it?
There was a problem hiding this comment.
Hi Volker,
"dim" is the dimensionality of a correction function. It is not known
until it is fetched from the database for the first event processed.
The purpose of the "mapFFTJet" method is to map jet variables
into the input variables of that correction function. "mapFFTJet" knows
how to map one variable only. There is no reasonable way in which this
particular mapper can do its job unless dim == 1, that is why
an exception is thrown. Thus dim != 1 is clearly a misconfiguration
problem -- most likely, a wrong correction function configured.
Regards,
Igor
On 05/12/2014 05:18 AM, Volker Adler wrote:
In JetMETCorrections/FFTJetObjects/interface/L2RecoScaleCalculator.h:
+class L2RecoScaleCalculator : public AbsFFTSpecificScaleCalculator
+{
+public:
- inline explicit L2RecoScaleCalculator(const edm::ParameterSet& ps)
: m_radiusFactor(ps.getParameter<double>("radiusFactor")) {}
- inline virtual ~L2RecoScaleCalculator() {}
- inline virtual void mapFFTJet(const reco::Jet& /* jet */,
const reco::FFTJet<float>& fftJet,const math::XYZTLorentzVector& /\* current */,double\* buf, const unsigned dim) const- {
if (dim != 1)throw cms::Exception("FFTJetBadConfig")Please do not throw at run time. An |edm::LogError| is preferred.
Where is |dim| determined? Is it possibly known at c'tor time already?
Beside that, it is not used in the function. So, why should the function break due to it?—
Reply to this email directly or view it on GitHub https://github.com/cms-sw/cmssw/pull/3810/files#r12521502.
There was a problem hiding this comment.
A possible solution could be to issue an edm::LogError and turn this into a bool function returning false in case of failure with a meaningful assignment to buf[0] -- or let the double be returned directly to be assigned in the calling function, which would also allow for a more flexible usage in the future.
There was a problem hiding this comment.
Hi Volker,
In this particular situation, there is nothing calling function
could do to improve the situation. The proper solution is
to fix the configuration file.
Issuing edm::LogError can potentially result in a large amount
of CPU time wasted -- in case it is ignored and the person
running the code only finds out about the problem later.
Regards,
Igor
On 05/12/2014 08:07 AM, Volker Adler wrote:
In JetMETCorrections/FFTJetObjects/interface/L2RecoScaleCalculator.h:
+class L2RecoScaleCalculator : public AbsFFTSpecificScaleCalculator
+{
+public:
- inline explicit L2RecoScaleCalculator(const edm::ParameterSet& ps)
: m_radiusFactor(ps.getParameter<double>("radiusFactor")) {}
- inline virtual ~L2RecoScaleCalculator() {}
- inline virtual void mapFFTJet(const reco::Jet& /* jet */,
const reco::FFTJet<float>& fftJet,const math::XYZTLorentzVector& /\* current */,double\* buf, const unsigned dim) const- {
if (dim != 1)throw cms::Exception("FFTJetBadConfig")A possible solution could be to issue an |edm::LogError| and turn this into a |bool| function returning |false| in case of failure with a meaningful assignment to |buf[0]| -- or let the |double| be
returned directly to be assigned in the calling function, which would also allow for a more flexible usage in the future.—
Reply to this email directly or view it on GitHub https://github.com/cms-sw/cmssw/pull/3810/files#r12526899.
There was a problem hiding this comment.
Do I understand correctly that dim is supposed to be constant in a given job, depending on its configuration, and that the exception will be thrown at the first event in any case?
There was a problem hiding this comment.
Yes, this is correct, "dim" s supposed to be constant in a given job.
In case of misconfiguration the exception will be thrown at the first event.
Regards,
Igor
On 05/12/2014 08:34 AM, Volker Adler wrote:
In JetMETCorrections/FFTJetObjects/interface/L2RecoScaleCalculator.h:
+class L2RecoScaleCalculator : public AbsFFTSpecificScaleCalculator
+{
+public:
- inline explicit L2RecoScaleCalculator(const edm::ParameterSet& ps)
: m_radiusFactor(ps.getParameter<double>("radiusFactor")) {}
- inline virtual ~L2RecoScaleCalculator() {}
- inline virtual void mapFFTJet(const reco::Jet& /* jet */,
const reco::FFTJet<float>& fftJet,const math::XYZTLorentzVector& /\* current */,double\* buf, const unsigned dim) const- {
if (dim != 1)throw cms::Exception("FFTJetBadConfig")Do I understand correctly that |dim| is supposed to be constant in a given job, depending on its configuration, and that the exception will be thrown at the first event in any case?
—
Reply to this email directly or view it on GitHub https://github.com/cms-sw/cmssw/pull/3810/files#r12528443.
There was a problem hiding this comment.
Ok, then we can consider this as thrown at initialisation time, which is "almost" at construction time.
There was a problem hiding this comment.
Yes, sure.
Igor
On 05/12/2014 09:37 AM, Volker Adler wrote:
In JetMETCorrections/FFTJetObjects/interface/L2RecoScaleCalculator.h:
+class L2RecoScaleCalculator : public AbsFFTSpecificScaleCalculator
+{
+public:
- inline explicit L2RecoScaleCalculator(const edm::ParameterSet& ps)
: m_radiusFactor(ps.getParameter<double>("radiusFactor")) {}
- inline virtual ~L2RecoScaleCalculator() {}
- inline virtual void mapFFTJet(const reco::Jet& /* jet */,
const reco::FFTJet<float>& fftJet,const math::XYZTLorentzVector& /\* current */,double\* buf, const unsigned dim) const- {
if (dim != 1)throw cms::Exception("FFTJetBadConfig")Ok, then we can consider this as thrown at initialisation time, which is "almost" at construction time.
—
Reply to this email directly or view it on GitHub https://github.com/cms-sw/cmssw/pull/3810/files#r12532045.
|
+1 |
…n-update AT -- Convenience utilities for configuring fftjet jet corrections.
Added a convenience class and a number of python configs