Skip to content
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

Add Gaudi algorithm to perform MVA calibration of cluster energies #68

Merged
merged 7 commits into from
Feb 29, 2024

Conversation

giovannimarchiori
Copy link
Contributor

@giovannimarchiori giovannimarchiori commented Feb 27, 2024

Implement a Gaudi algorithm that reads a model via ONNX runtime and calibrates the clusters
Pre-trained models for ALLEGRO's ECAL will be uploaded in the LAr_scripts package (and FCC-config)

In principle the code could also run a calibration for ECAL+HCAL clusters, if it is trained for that purpose and makes sense.

I put this in RecFCCeeCalorimeter since I'm working on the ee machine but I think the code is general enough that could be promoted to RecCalorimeter.

Also, the code calculates in input the cluster energies per layers from the cells. Since this information is needed also by other algorithms, it would make sense as a next step to factorise this part into a separate algorithms that decorates the clusters in memory with some additional variables (energy fractions, shower shapes, ..) that are available to all subsequent algorithms.

Tagging @BrieucF

@giovannimarchiori
Copy link
Contributor Author

Hi @kjvbrt ,
thanks for your careful review!
I implemented all your suggestions in the latest commit
cheers,
Giovanni

@BrieucF
Copy link
Contributor

BrieucF commented Feb 29, 2024

This is really great to have! Thanks Giovanni!

For the trained model, I propose to centralize them here /eos/project/f/fccsw-web/www/filesForSimDigiReco/ALLEGRO/ALLEGRO_o1_v02/, following the same versioning/path conventions as in k4geo

@giovannimarchiori
Copy link
Contributor Author

Hi @BrieucF , wouldn't it be better to put the files in GitHub, so that we use its versioning system, record changes in the commit messages, can track more easily when files were updated, and so on? They are small files, they are not going to make the repository too big

@BrieucF
Copy link
Contributor

BrieucF commented Feb 29, 2024

If they are small enough why not

@giovannimarchiori
Copy link
Contributor Author

Those from LGBM, which has a performance very similar to XGB, are 300-400k. Those from XGB are about 2.5M

@BrieucF
Copy link
Contributor

BrieucF commented Feb 29, 2024

On the other hand the diff of those files will always be a "full diffs" so not sure the gitHub versioning really brings something more than having our models as model_v1, model_v2, etc. It will pollute the gitHub history and if we introduce bigger models at some point we will end up with heterogenous storage places. NB: the files from /eos/project/f/fccsw-web/www/filesForSimDigiReco/ALLEGRO/ALLEGRO_o1_v02/ can be retrieved with wget or curl, e.g.:

curl -O -L http://fccsw.web.cern.ch/fccsw/filesForSimDigiReco/ALLEGRO/ALLEGRO_o1_v02/elecNoise_ecalBarrelFCCee_theta.root

@BrieucF
Copy link
Contributor

BrieucF commented Feb 29, 2024

But ok, we can decide later, this should not prevent us to merge this PR

@kjvbrt kjvbrt merged commit c432481 into HEP-FCC:main Feb 29, 2024
3 of 6 checks passed
jmcarcell added a commit to key4hep/key4hep-spack that referenced this pull request Mar 2, 2024
@giovannimarchiori giovannimarchiori deleted the gmarchio-main-20240222 branch April 18, 2024 15:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants