-
Notifications
You must be signed in to change notification settings - Fork 151
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
[TRD]: Check on hc2d #2152
[TRD]: Check on hc2d #2152
Conversation
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.
Thanks for the work Deependra, but I think we need to refine the check a bit. And please make check if other checkers are not using the DPL CCDB fetching mechanism as well.
Modules/TRD/src/CheckOnHc2d.cxx
Outdated
ILOG(Debug, Support) << "configure() : using default timestam of now = " << mTimestamp << ENDM; | ||
} | ||
auto& mgr = o2::ccdb::BasicCCDBManager::instance(); | ||
mgr.setTimestamp(mTimestamp); |
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.
Should checks still use the BasicCCDBManager? Should one not use the DPL CCDB fetching mechanism instead? This we do for example in the TrackletsTask or DigitsTask
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.
As I discussed with Piotr, He told me this is not possible.
Modules/TRD/src/CheckOnHc2d.cxx
Outdated
double content = h->GetBinContent(i, j); | ||
if (content == 0) { | ||
result1 = Quality::Bad; | ||
result1.addReason(FlagReasonFactory::Unknown(), "some half chambers are empty"); |
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.
But if the bin is empty and the half-chamber is already flagged as bad, then the quality should not be bad. It should still be good.
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.
I tried to address this issue. Please let me know something is wrong in new commit.
Modules/TRD/src/CheckOnHc2d.cxx
Outdated
if (TRDHelpers::isHalfChamberMasked(hcid, mChamberStatus)) { | ||
ILOG(Debug, Trace) << "Masked half Chamber id =" << hcid << ENDM; | ||
result2 = Quality::Bad; | ||
result2.addReason(FlagReasonFactory::Unknown(), "some chmabers are masked"); |
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.
For some masked chambers everything is still OK. We will always have that, so your check would never yield good quality. I would change quality to bad based on masked chambers only if we have either a full sector masked or more than 40% of all half-chambers masked from DCS side
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.
I tried to address this also. let me know if something wrong.
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.
…40% of chambers are masked or any entire sector is masked
Hi @deependra170598 do you have time for a debug session maybe tomorrow at some point? Concerning the timestamp I saw this code in MFT where they obtain the time directly from the MO. That makes more sense to me then setting it as parameter.. QualityControl/Modules/MFT/src/QcMFTDigitCheck.cxx Lines 131 to 134 in 8991a03
|
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.
I am adding some small comments now. But I think we can also go through this PR together next week
namespace o2::quality_control_modules::trd | ||
{ | ||
|
||
void CheckOnHc2d::configure() |
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.
do we need this method at all?
if (TRDHelpers::isHalfChamberMasked(hcid, mChamberStatus)) { | ||
ILOG(Debug, Trace) << "Masked half Chamber id =" << hcid << ENDM; | ||
nMaskedHC++; | ||
if (nMaskedHC > (NCHAMBER * 2 * 0.4)) { // if nMaskedHC is greator than 40% of all chambers |
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.
The 40% should be a parameter that we can change from the json file
auto* h = dynamic_cast<TH2F*>(mo->getObject()); | ||
|
||
if (checkResult == Quality::Good) { | ||
h->SetFillColor(kGreen); |
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.
Does one see it if the histogram fill color for a TH2 changes if we draw it with COLZ option? I think we need a text box here
Right now I am closing this PR. I will first merge TrackletsCheck and TrackletCountsCheck then will work on 2DHC check. |
Hi @martenole and others!
In this PR, I tried to implement check on Half Chamber status.
It contains two sub checks...
trackletsperHC2D
is emptyAny masked half-chamber or empty chamber results in bad quality otherwise good.