[PWGEM,Tools] Update EMCal Conversion ML Model and enable batching of…

[mhemmer-cern]

… ml models

• Add option to use ml models in batches, instead of testing one candidate per call to reduce speed up application of ml models
• Update emcalPhotonMcTask.cxx to incoporate these changes
• Update histograms that will be stored and filled

[github-actions]

O2 linter results: ❌ 0 errors, ⚠️ 0 warnings, 🔕 2 disabled

[mhemmer-cern]

Hello @fcatalan92 @fmazzasc
While working on some ML model for EMCal cluster, I tried adding the option to use the ml output in batches. Also looking into model.h it looked to me like there is a potential memory issue where a pointer gets created pointing to a local variable inside the function. After return is called the memory for the variable should be freed making the pointer point to garbage.
Could you please have a look into the changes and the comment made by this commit to the ML code?

[fmazzasc]

Hi @mhemmer-cern , the PR looks good to me and I am approving it for the ML side. @ChSonnabend , could you maybe check the memory issue in model.h? It seems to me that @mhemmer-cern is right

[ChSonnabend]

Hi all,
Yes Marvin is right and I discussed already with him privately about the issue. It is not an issue if the output of the model is read directly after execution because the memory is not overwritten. However it is correct that it should be fixed. I wrote this code several years ago but will fix it in the near future. As far as I am aware not many other people are using model.h as it was replaced by MlResponse.h (which I never knew existed so far). I can't give a direct ETA but will try to fix it this week still. Thanks for finding it @mhemmer-cern

[alibuild]

Auto-approving on behalf of @mhemmer-cern.

[fcatalan92]

Hi @mhemmer-cern, thanks a lot for the developments and for spotting this issue!

@ChSonnabend MlResponse is built on top of model and directly call its functions (it is meant as a wrapper for analysis). So this issue affects all the analyses relying on computing the ML inference on the GRID and using MlResponse, which should be the vast majority since this is the recommended way to go. I'm pretty sure everything works at the moment given no bogus results were observed using the approach, but thanks a lot to take care of fixing this issue!

I guess that returning the outputValues by copy should be enough. OnnxRuntime also implements the possibility to return the value in a user-provided output https://github.com/microsoft/onnxruntime/blob/bb331b7a235435863a89f4bfcc3b2138928026ec/include/onnxruntime/core/session/onnxruntime_cxx_api.h#L1961 .