correction to check function

[deependra170598]

Correcting check function but still investigating trend problem

[deependra170598]

Hi @knopers8 and @martenole, May you review this PR?

[martenole]

to avoid these kind of typos wouldn't it be better to use two variables in your checker class, e.g. mThresholdMeanLow and mThresholdMeanHigh or something like that? And then for reading the parameters from json file I'd prefer you use the common getFromConfig as it is done for example in

QualityControl/Modules/TRD/src/DigitsTask.cxx

Lines 408 to 415 in a0dc3f4

	// this is how to get access to custom parameters defined in the config file at qc.tasks.<task_name>.taskParameters
	mDriftRegion.first = getFromConfig<float>(mCustomParameters, "driftRegionStart", 7.f);
	mDriftRegion.second = getFromConfig<float>(mCustomParameters, "driftRegionEnd", 20.f);
	mPulseHeightPeakRegion.first = getFromConfig<float>(mCustomParameters, "peakRegionStart", 0.f);
	mPulseHeightPeakRegion.second = getFromConfig<float>(mCustomParameters, "peakRegionEnd", 5.f);
	mPulseHeightThreshold = getFromConfig<unsigned int>(mCustomParameters, "phThreshold", 400u);
	mChambersToIgnore = getFromConfig<string>(mCustomParameters, "ignoreChambers", "16_3_0");
	mLayerLabelsIgnore = getFromConfig<bool>(mCustomParameters, "ignoreLabels", true);

. You'll need to include Common/Utils.h for this

[deependra170598]

I updated this PR with the above suggestions.

[martenole]

what are you using the BasicCCDBManager for?

[deependra170598]

I try to explain how I understood the concept of BasicCCDBManager. Although I do not fully understand.

1. instance() method of class BasicCCDBManager : public CCDBManagerInstance initialise the class with ccdb server.
2. Then setTimestamp() method is used for setting timestamp manually if required to be given from config file otherwise current time stamp is used. It set timestamp cache for all queries (line no. 75 of BasicCCDBManager.h).
3. I see that BasicCCDBManager class itself by default set current timestamp (line no. 189 of BasicCCDBManager.h), so if we remove it from checker class it run without any problem.
4. In lack of knowledge, I kept these line because I did not know its consequences.
   I will do how you suggest.

[deependra170598]

I had in my mind that in background of workflow, It set the timestamp of QO object generated by checker class. And if a specific object is needed by any postprocessing in future, It can use that timestamp to query that QO object. Although this thought needs validation.

[martenole]

Hi Deependra, I would simply remove the CCDB manager completely from your task, because to me it does not look like you are using it. In some cases people are requesting CCDB objects in their tasks and use the CCDB manager for it, but I don't think you need any calibration objects for your check so I would get rid of it to keep the code as clean as possible. Later we will need some CCDB objects for more advanced checks, but also there not the CCDB manager should be used anymore, but the more modern DPL CCDB fetcher. But we can talk about this when we get there :)

[deependra170598]

Hi Ole, I removed that part.

[deependra170598]

Hi @knopers8 , May you look at it if any review is required from your side?

[knopers8]

macOS CI errors seem legit:

/Volumes/build/alice-ci-workdir/qualitycontrol-o2/sw/SOURCES/QualityControl/1970/0/Modules/TRD/src/TrackletPerTriggerCheck.cxx:69:119: error: format specifies type 'long' but the argument has type 'int' [-Werror,-Wformat]
        msg1->AddText(TString::Format("Hist Can't be ignored. Stat is enough. Entries: %d > Threshold: %ld", Entries, mStatThreshold));
                                                                                                       ~~~            ^~~~~~~~~~~~~~
                                                                                                       %d
--
        msg1->AddText(TString::Format("Hist Can't be ignored. Stat is enough. Entries: %d > Threshold: %ld", Entries, mStatThreshold));
                                                                                                       ~~~            ^~~~~~~~~~~~~~
                                                                                                       %d
/Volumes/build/alice-ci-workdir/qualitycontrol-o2/sw/SOURCES/QualityControl/1970/0/Modules/TRD/src/TrackletPerTriggerCheck.cxx:72:114: error: format specifies type 'long' but the argument has type 'int' [-Werror,-Wformat]
        msg1->AddText(TString::Format("Hist Can be ignored. Stat is low. Entries: %d < Threshold: %ld", Entries, mStatThreshold));
                                                                                                  ~~~            
...

[martenole]

Thanks, should be good to merge now