Fork me on GitHub

Opened 9 years ago

Last modified 2 years ago

#728 new Bug

UniqueObjectFinder processes in arbitrary order

Reported by: Chase Owned by:
Priority: critical Milestone:
Component: Delphes code Version: Delphes 3
Keywords: Cc:

Description

Hi,

The following bug is based on the current master branch, but presumably affects all versions since UniqueObjectFinder was implemented (i.e. d7d2da362 from Apr 2013).

Recently I've been having trouble with photon objects randomly going missing. I traced the issue to the UniqueObjectFinder module. In my card, photons were listed first (which should mean they are given precedence over all objects), but instead I found that only when I listed photons _after_ jets, would they show up in the output list.

I did some digging in the code and I believe there is a significant bug in the implementation:
https://github.com/delphes/delphes/blob/master/modules/UniqueObjectFinder.cc#L111

This implementation appears to assume that std::map iterators cycle through keys in the order in which they were added to the map. This is *not* in general the case, see e.g.:
http://www.cplusplus.com/reference/map/map/begin/

"Because map containers keep their elements ordered at all times, begin points to the element that goes first following the container's sorting criterion."

By adding some std:cout statements, I was able to confirm that the order in which keys were processed was not the same as the order in which they were added. Since I have only recently been having trouble with this, I assume the behavior is compiler- or otherwise system-dependent. To me this seems unacceptable, and this module should be reimplemented to use an explicitly-ordered data structure.

-Chase Shimmin

Change History (2)

comment:1 by Pavel Demin, 9 years ago

Hi,

Thanks a lot for finding this bug!

I've just replaced map with vector in 93da593af91a3f3ebded2ef3af70b2af337f6d97/git.

Could you please check if it solves the problem?

Best regards,

Pavel

comment:2 by Chase, 9 years ago

Hi Pavel,
Thanks for the quick fix! I just cherry-picked your change and things seem to be working as expected. I'd suggest to consider backporting this to older releases as a patch (I believe MadGraph is still distributing 3.1.x, last time I checked).

Cheers,
-Chase Shimmin

Version 0, edited 9 years ago by Chase (next)
Note: See TracTickets for help on using tickets.