[AFIT-109] Add MIP peak monitoring for FT0 #2552
Conversation
|
@afurs, regarding |
There was a problem hiding this comment.
@afurs, regarding
vector<bool>, I agree that abitsetwould be technically better. But I think that from the user/dev perspective it's nicer to usevector<bool>so that the check code becomes more unified and easier to read/understand/develop, also for more novice developers. In practice I guess this increases the used memory with ~1 byte per list parameter, so I wouldn't say it's a problem.
@andreasmolander I fully disagree, memory increase is the less problem. As I sad before, std::vector<bool> is bad practice due to several reasons. For example (this is not an only problem which may raise), try:
std::vector<bool> vec{true,false,true};
auto &el= vec[0];
I dont know, if for other reviewers(@knopers8 @Barthelemy ) it is ok, then no problem, lets merge it. But I strongly recommend to replace it by bitsetor boost::dynamic_bitset
There was a problem hiding this comment.
@andreasmolander ok, anyway it is in dedicated task, so in case of problems it can be turned off.
| parseableParam = mCustomParameters.atOrDefaultValue("gausParamsSigma", "3.0, 7.0"); | ||
| mGausParamsSigmas = helper::parseParameters<float>(parseableParam, ","); | ||
|
|
||
| parseableParam = mCustomParameters.atOrDefaultValue("meanWarningsLow", ""); | ||
| mMeanWarningsLow = helper::parseParameters<float>(parseableParam, ","); | ||
|
|
||
| parseableParam = mCustomParameters.atOrDefaultValue("meanWarningsHigh", ""); | ||
| mMeanWarningsHigh = helper::parseParameters<float>(parseableParam, ","); | ||
|
|
||
| parseableParam = mCustomParameters.atOrDefaultValue("meanErrorsLow", ""); | ||
| mMeanErrorsLow = helper::parseParameters<float>(parseableParam, ","); | ||
|
|
||
| parseableParam = mCustomParameters.atOrDefaultValue("meanErrorsHigh", ""); | ||
| mMeanErrorsHigh = helper::parseParameters<float>(parseableParam, ","); | ||
|
|
||
| parseableParam = mCustomParameters.atOrDefaultValue("sigmaWarnings", ""); | ||
| mSigmaWarnings = helper::parseParameters<float>(parseableParam, ","); | ||
|
|
||
| parseableParam = mCustomParameters.atOrDefaultValue("sigmaErrors", ""); | ||
| mSigmaErrors = helper::parseParameters<float>(parseableParam, ","); | ||
|
|
||
| parseableParam = mCustomParameters.atOrDefaultValue("drawMeanWarningsLow", "false, false"); | ||
| mDrawMeanWarningsLow = helper::parseParameters<bool>(parseableParam, ","); | ||
|
|
||
| parseableParam = mCustomParameters.atOrDefaultValue("drawMeanWarningsHigh", "false, false"); | ||
| mDrawMeanWarningsHigh = helper::parseParameters<bool>(parseableParam, ","); | ||
|
|
||
| parseableParam = mCustomParameters.atOrDefaultValue("drawMeanErrorsLow", "true, true"); | ||
| mDrawMeanErrorsLow = helper::parseParameters<bool>(parseableParam, ","); | ||
|
|
||
| parseableParam = mCustomParameters.atOrDefaultValue("drawMeanErrorsHigh", "true, true"); | ||
| mDrawMeanErrorsHigh = helper::parseParameters<bool>(parseableParam, ","); | ||
|
|
||
| parseableParam = mCustomParameters.atOrDefaultValue("drawSigmaWarnings", "false, false"); | ||
| mDrawSigmaWarnings = helper::parseParameters<bool>(parseableParam, ","); | ||
|
|
||
| parseableParam = mCustomParameters.atOrDefaultValue("drawSigmaErrors", "false, false"); | ||
| mDrawSigmaErrors = helper::parseParameters<bool>(parseableParam, ","); | ||
|
|
||
| parseableParam = mCustomParameters.atOrDefaultValue("labelPos", "0.15, 0.2, 0.85, 0.45"); | ||
| mVecLabelPos = helper::parseParameters<double>(parseableParam, ","); |
There was a problem hiding this comment.
btw, you can patch parseParameters method and use mCustomParameters.atOrDefaultValue inside. Less code lines
| "meanWarningsLow": "12, 26", | ||
| "meanWarningsHigh": "16, 30", | ||
| "meanErrorsLow": "10, 24", | ||
| "meanErrorsHigh": "18, 32", | ||
| "sigmaWarnings": "4, -1", | ||
| "sigmaErrors": "5, -1", | ||
| "drawMeanWarningsLow": "false, false", | ||
| "drawMeanWarningsHigh": "false, false", | ||
| "drawMeanErrorsLow": "true, true", | ||
| "drawMeanErrorsHigh": "true, true", | ||
| "drawSigmaWarnings": "false, false", | ||
| "drawSigmaErrors": "fasle, false", |
There was a problem hiding this comment.
I'm not sure but maybe it is better to use and parse list of json entities, smth like:
"peaks": [
{
"meanWarningsLow": 12,
"meanWarningsHigh": 16,
....
"draw_options": ["drawMeanErrorsLow", "drawSigmaWarnings"]
},
{
...
}
]
and parse it via boost property tree or RapidJSON. Then no need in vec of bools, you will just need to enumerate list of drawing options
There was a problem hiding this comment.
Yes that might be a good idea, let me think about it for the next iteration. One benefit (maybe?) of the current approach is that one can tell what the options are, just by looking at the config, and not having to jump over to the documentation...
|
I approved. The rest will come later, we need this task urgently... |
4 participants
No description provided.