JeffersonLab / analyzer

HallA C++ Analyzer
BSD 3-Clause "New" or "Revised" License
7 stars 54 forks source link

Simplest way to fix the Compile complaint for an Eye variable is to n… #134

Closed rwmichaels closed 7 years ago

rwmichaels commented 7 years ago

…ot Compile, which is ok since its not really a formula

rwmichaels commented 7 years ago

Fixing "eye" problem -- : Unknown name :[0]. Easiest solution is to not Compile if the THaVform is an "eye" variable. Reason we don't need to compile is that when the value is obtained, it just returns the index (plus a possible offset), and does NOT compute a THaFormla.
Double_t THaVform::GetData(Int_t index) const { if (IsEye()) return (Double_t)(index + fEyeOffset); // otherwise evaluate the corresponding THaFormula } More on the story: Histograms axes are, in most cases, THaFormula (or vectors of these), to be general. However, the "eye" was a special case. As a mnemonic that this was an "eye" I passed the formula "[0]" to the c'tor. (It looks like an eye when you rotate it, doesn't it ?) I think this formerly passed without complaint (??) but at present THaFormula::CheckBlacklistedNames issues the aforementioned complaint, which, however, has no consequence. The above fix avoids the complaint. I guess I could also make an "eye" not be a THaFormula, since it really isn't, but that would be more work.

hansenjo commented 7 years ago

Sounds good. Indeed, the "eye" doesn't behave like a THaFormula. Not compiling them should fix the compilation problem, by definition, but also points to a bit of design awkwardness. Well, if it works ...

BTW, THaFormulas with parameters, like "[0]", are ill-defined because the concept of parameters doesn't make sense for them. In fact they crash, so that's why explicit parameters as well as a few other expressions that implicitly use parameters are rejected. If "[0]" worked at some time in the past, it was sailing through "undefined behavior" land. There's still an open bug report on that that I have only partly addressed.

I'll merge the pull request later tonight.

Ole

Am 14.07.2017 um 15:11 schrieb Robert Michaels notifications@github.com:

Fixing "eye" problem -- THaVform::Compile: Unknown name :[0]. Easiest solution is to not Compile if the THaVform is an "eye" variable. Reason we don't need to compile is that when the value is obtained, it just returns the index (plus a possible offset), and does NOT compute a THaFormla. Double_t THaVform::GetData(Int_t index) const { if (IsEye()) return (Double_t)(index + fEyeOffset); // otherwise evaluate the corresponding THaFormula } More on the story: Histograms axes are, in most cases, THaFormula (or vectors of these), to be general. However, the "eye" was a special case. As a mnemonic that this was an "eye" I passed the formula "[0]" to the c'tor. (It looks like an eye when you rotate it, doesn't it ?) I think this formerly passed without complaint (??) but at present THaFormula::CheckBlacklistedNames issues the aforementioned complaint, which, however, has no consequence. The above fix avoids the complaint. I guess I could also make an "eye" not be a THaFormula, since it really isn't, but that would be more work.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or mute the thread.