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

MultiCluster-CaloParticle associator #32941

Closed
wants to merge 1,190 commits into from

Conversation

lecriste
Copy link
Contributor

PR description:

This PR introduces an associator that can be consumed by several analyzers, removing code duplication. The first replacement is made for the HGVHistoProducerAlgo::fill_multi_cluster_histos method, no changes are expected in the output.

PR validation:

runTheMatrix -l limited

@cmsbuild
Copy link
Contributor

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-32941/21171

  • This PR adds an extra 76KB to repository

Code check has found code style and quality issues which could be resolved by applying following patch(s)

@slava77
Copy link
Contributor

slava77 commented Feb 18, 2021

Code check has found code style and quality issues which could be resolved by applying following patch(s)

please follow up; we can not do further tests without it

@slava77
Copy link
Contributor

slava77 commented Feb 25, 2021

Code check has found code style and quality issues which could be resolved by applying following patch(s)

please follow up; we can not do further tests without it

@slava77
Copy link
Contributor

slava77 commented Feb 25, 2021

@lecriste
(tagging explicitly, in case the earlier messages got lost)

@lecriste
Copy link
Contributor Author

Hi Slava, after opening the PR I found a corner case in which results differ from original code.
I would push the code-checks patch together with the fix commit once I have it.
Please let me know if that's not OK with you.

@slava77
Copy link
Contributor

slava77 commented Feb 25, 2021

Please let me know if that's not OK with you.

OK.
I was just pinging to make sure you received the message(s).

@slava77
Copy link
Contributor

slava77 commented Mar 11, 2021

@lecriste
please clarify on the current status of the PR
Thank you.

@lecriste
Copy link
Contributor Author

We decided to change a reco DataFormat (the Trackster) along with a SimDataFormat, in order to proceed with the implementation of the associator in this PR and, at the same time, keep the current feature of a dedicated validation per TICL iteration.
I am using this branch to test locally the DataFormat change, once happy I will open a separated PR for that and then come back to this one.

@slava77
Copy link
Contributor

slava77 commented Apr 5, 2021

@lecriste
please clarify on the status and plans for this PR.
Thank you.

@lecriste
Copy link
Contributor Author

lecriste commented Apr 5, 2021

I need to rebase it to the other merged/open PRs and adapt the code to use the new Trackster data format.
Will do by next week.

@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 9, 2021

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-32941/21980

Code check has found code style and quality issues which could be resolved by applying following patch(s)

@lecriste
Copy link
Contributor Author

lecriste commented Apr 9, 2021

This PR is superseded by #33384, which contains only the associator implementation.
We do not use it in Validation/HGCalValidation/src/HGVHistoProducerAlgo.cc because we are going to replace MultiCluster with Trackster therein.

@lecriste lecriste closed this Apr 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment