-
Notifications
You must be signed in to change notification settings - Fork 549
[PWGHF] Patches for first Xic autoencoder study #10813
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
base: master
Are you sure you want to change the base?
Conversation
Please consider the following formatting changes to AliceO2Group#10813
Hi @macamerl, Thanks for the development! First of all, let me ping the code owner to see if they have any comments on this implementation. @mfaggin @cterrevo and @phymanshu. Then I only have a few comments, see the review. Then could you also fix magalinter errors since they come from this PR? |
changes to fix megalinter check
changes to fix megalinter checks
Changes to fix the PR checks
Hi @macamerl @zhangbiao-phy |
Thanks @phymanshu. Yes, I locally checked that the BDT inference has no problem after the changes that I made. The flags "applyMSE" and "applyMinMax" are set false/0 by default. In any case, I will privately send you the config file that I used for the check. |
//---- temporary MSE function | ||
float MSE(std::vector<float> y_true, std::vector<float> y_pred, std::vector<float> ScaleMin, std::vector<float> ScaleMax, bool scaling) | ||
{ | ||
float mse = 0.0f; | ||
float sum = 0.0f; | ||
|
||
std::vector<float> yf_true; | ||
if (y_true.size() != y_pred.size()) { | ||
LOG(debug) << "size of input vector =" << y_true.size(); | ||
LOG(debug) << "size of AE output vector =" << y_pred.size(); | ||
LOG(fatal) << "vectors of input and predictions don't have the same size"; | ||
} else { // MSE | ||
for (size_t j = 0; j < y_pred.size(); ++j) { // for over the features | ||
if (scaling == true) { // MinMax scaling | ||
LOG(debug) << "--------------> MinMax scaling Debug \t" << ScaleMin.at(j) << "\t" << ScaleMax.at(j); | ||
yf_true.push_back((y_true.at(j) - ScaleMin.at(j)) / (ScaleMax.at(j) - ScaleMin.at(j))); | ||
LOG(debug) << "feature = " << j << " ----> input = " << y_true.at(j) << " scaled feature = " << yf_true.at(j); | ||
} else { | ||
yf_true.push_back(y_true.at(j)); | ||
} | ||
sum += pow(((yf_true).at(j) - (y_pred).at(j)), 2); // has dimensions | ||
LOG(debug) << "feature = " << j << " ----> input = " << yf_true.at(j) << " AE prediction = " << y_pred.at(j); | ||
} | ||
mse = sum / y_pred.size(); // MSE of a candidate | ||
} |
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.
Since this is not HF specific, I would move this in the ML folder. You could consider to add it as a function of the https://github.com/AliceO2Group/O2Physics/blob/master/Tools/ML/MlResponse.h class, or to add a header file with utils functions.
Please also follow the naming conventions of O2Physics when you move it (camel case), thanks!
if (applyMSE) { | ||
hfMlXicToPKPiCandidate(outputMSEXicToPKPi, outputMSEXicToPiKP); | ||
} else { | ||
hfMlXicToPKPiCandidate(outputMlXicToPKPi, outputMlXicToPiKP); | ||
} |
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.
Can you not avoid all the branching and just make the decision once?
Minimal changes in XicToPKPi specific files (task and selector) to evaluate the MSE between input (externally scaled) and internal scaled output of autoencoder. At moment, one can not run Autoencoder and BDT at the same time since they competitively fill candidate.mlProbXicToPKPi()[0].