Closed GaelTouquet closed 5 years ago
cbernet
commented
5 years ago You're deleting a lot of IDs in Electron.py that don't have a VID-equivalent but are used in other analysis.
Yes, I had to do a major clean up of the infrastructure, there was a lot of obsolete / illogical code in this class. I think I've added all the IDs supported by e/gamma. I guess the new infrastructure can accommodate the other IDs. But since we're not using them, it would be difficult for us to implement them.
peruzzim
commented
5 years ago @cbernet actually I'm afraid that many of those ID are needed, as they are used e.g. in SUSY or in ongoing versions of other analyses that have not updated to the latest ID versions
cbernet
commented
5 years ago @peruzzim Yes, no problem, I'm sure they can be re-added easily in the new infrastructure. But as I said, it would be difficult for us to do it as we are not using these ids. Maybe you guys can survey which IDs are still needed and re-add them after the merge? I will be happy to assist and explain how the new infrastructure is working.
The reason why I had to do all this is that we could not re-code the v2 electron ID for heppy, as was done in the past. Instead, @guitargeek and myself teamed up to provide official egamma tools to run the ID in FWLite. So now, we simply use the e/gamma cmssw tools in FWLite. This makes it much easier for us and, in the future, we will get the new IDs almost for free.
Getting v2 to work was actually quite complicated, we spent maybe a month on this.
To make it work, we had to redesign the ID interface in Heppy, because it was really chaotic. I guess that it evolved over the years without any real code review. So we took the easy path to remove the IDs that were not supported by e/gamma, as I thought that no analysis would be approved with an unsupported ID anyway. Also, we thought that this fork was kind of dead, so we decided to just maintain our own version. This development was already picked up by @bachtis from our fork, as far as I know.
Now the electron ID is obtained through a single easy method, using normalized names, like this:
gpetruc
commented
5 years ago I think the only way forward at the moment is to keep both the old (by hand) and new (VID-based) backends for the id. There's some parts of the old implementation (all run1-related IDs) that can be safely dropped, but many run2 ones exist and don't necessarily have the VID implementation (e.g. even just for the cut-based IDs, VID doesn't provide also the working points for ID-only without also the isolation cut).
cbernet
commented
5 years ago Yes, I think it’s a good solution.
On 12 Apr 2019, at 15:06, Giovanni Petrucciani notifications@github.com<mailto:notifications@github.com> wrote:
I think the only way forward at the moment is to keep both the old (by hand) and new (VID-based) backends for the id. There's some parts of the old implementation (all run1-related IDs) that can be safely dropped, but many run2 ones exist and don't necessarily have the VID implementation (e.g. even just for the cut-based IDs, VID doesn't provide also the working points for ID-only without also the isolation cut).
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://github.com/CERN-PH-CMG/cmg-cmssw/pull/749#issuecomment-482567217, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AD8ku37C8wIk7sGNJfP-LMSrewHg5fNvks5vgITTgaJpZM4crsry.
Most of the changes come from merging code from Egamma, as can be seen in this issue and this merged pull request. Most changes from us are in PhysicsTools/Heppy . Also it seems some tau changes have slipped from this commit about the TauID (although I don't understand why they are shown here even though the PR was merged...). Feel free to review and suggest changes.
@cbernet @steggema @lucastorterotot