CTP: change in the task, check and trending #2454
Conversation
| std::string mTrendedClassNames[5] = { "classContentMinBias1:time", "classContentMinBias2:time", "classContentDMC:time", "classContentEMC:time", "classContentPHO:time" }; | ||
| std::string mTrendedInputRatioNames[4] = { "inputContentMinBias2/inputs", "inputContentDMC/inputs", "inputContentEMC/inputs", "inputContentPHO/inputs" }; | ||
| std::string mTrendedClassRatioNames[4] = { "classContentMinBias2/classes", "classContentDMC/classes", "classContentEMC/classes", "classContentPHO/classes" }; | ||
| std::string ctpinputs[49] = { "MT0A", "MT0C", "MTVX", "MTSC", "MTCE", "MVBA", "MVOR", "MVIR", "MVNC", "MVCH", "11", "12", "0UCE", "0DMC", "0USC", "0UVX", "0U0C", "0U0A", "COS", "LAS", "0EMC", "0PH0", "23", "24", "1ZED", "1ZNC", "1PHL", "1PHH", "1PHM", "30", "31", "32", "33", "34", "35", "36", "0EJ1", "0EG1", "0EJ2", "0EG2", "DJ1", "DG1", "DJ2", "DG2", "45", "46", "47", "EMBA", "49" }; // all ctp input names |
There was a problem hiding this comment.
The inputs already exists in:
https://github.com/AliceO2Group/AliceO2/blob/bdde90491b05acd2505b67be723eda13033be949/DataFormats/Detectors/CTP/src/Configuration.cxx#L1071
Can not this beused ?
| std::vector<int> mVecIndexBad; // vector of ctp input and class indices, which had a big relative change | ||
| std::vector<int> mVecIndexMedium; // vector of ctp input and class indices, which had a relative change | ||
| std::bitset<o2::constants::lhc::LHCMaxBunches> mLHCBCs; // LHC filling scheme | ||
| const char* ctpinputs[49] = { "T0A", "T0C", "TVX", "TSC", "TCE", "VBA", "VOR", "VIR", "VNC", "VCH", "11", "12", "UCE", "DMC", "USC", "UVX", "U0C", "U0A", "COS", "LAS", "EMC", "PH0", "23", "24", "ZED", "ZNC", "PHL", "PHH", "PHM", "30", "31", "32", "33", "34", "35", "36", "EJ1", "EJ2", "EG1", "EG2", "DJ1", "DG1", "DJ2", "DG2", "45", "46", "47", "48", "49" }; // ctp input names |
There was a problem hiding this comment.
The inputs already exists in:
https://github.com/AliceO2Group/AliceO2/blob/bdde90491b05acd2505b67be723eda13033be949/DataFormats/Detectors/CTP/src/Configuration.cxx#L1071
Can not this beused ?
There was a problem hiding this comment.
@lhusova can you please consider implementing my additional suggestions? Apart from that, for me it is ok.
The only general comment I have is that the code in the generatePlots() function is still quite complicated, you might try to simplify the loop over the indexes in afuture PR.
| } | ||
| if (input2Tokens.size() > 1) { | ||
| mScaleInput2 = std::stod(input2Tokens[1]); | ||
| } |
There was a problem hiding this comment.
@lhusova you could allow also input1 to have an extra scaling factor. Maybe to be kept in mind for an additional PR
Modules/CTP/src/CTPTrendingTask.cxx
Outdated
| try { | ||
| mClassNames[0] = std::stof(mCustomParameters.at("minBias1Class", "default")); | ||
| } catch (const std::exception& e) { | ||
| mClassNames[0] = getFromExtendedConfig<string>(t.activity, mCustomParameters, "minBias1Class", "CMTVX-B-NOPF"); |
There was a problem hiding this comment.
You could write
mClassNames[0] = getFromExtendedConfig<string>(t.activity, mCustomParameters, "minBias1Class", mClassNamesDefault[0]);
to avoid a risk of mismatch in the default value.
Also valid for the other lines below.
| @@ -40,30 +40,30 @@ void RawDataReaderCheck::configure() | |||
| // reading the parameters from the config.json | |||
| // if not available, setting a default value | |||
| // the threshold values are nSigma | |||
| std::string param = mCustomParameters.atOrDefaultValue("thresholdRateBad", "3"); | |||
| std::string param = mCustomParameters["thresholdRateBad"]; | |||
There was a problem hiding this comment.
I would suggest to to use getFromExtendedConfig() also here.
There was a problem hiding this comment.
Here in the configure function, there is no information about the activity. If we would like to use getFromExtendedConfig(), I would suggest to move the read out of the parameters to the RawDataReaderCheck::startOfActivity. What do you think?
There was a problem hiding this comment.
@lhusova there is also a getFromConfig() function that retrieves the values from the old taskParameters, and which does not require to specify the activity.
lhusova
requested review from
Barthelemy,
knopers8 and
justonedev1
as code owners
* change of the parameter read out in Raw QC task * change of the parameter read out in the check * change of the parameter read out + consistent input names * trending plots generated based on the varexp * add token also for MB input1 * simplification and assuring of default names * histogram labels from CTPInputsConfiguration * change of the read out of the parameters in Check * fix --------- Co-authored-by: Lucia Anna Tarasovicova <[email protected]>
@aferrero2707 @lietava could you have a look? I changed all the things as discussed yesterday. With the new approach of the parameters read-out it was working for my local tests.
I also changed the generation of the trending plots. Now it does not relay on the order, how they are created in the confing. They are generated now based on the trended variable.