Use std::lower_bound to add/search for userFloats/Ints/Cands/KinResolutions (76X) #11574
Conversation
lgray
changed the title
|
@cmsbuild please test |
|
The tests are being triggered in jenkins. |
|
A new Pull Request was created by @lgray (Lindsey Gray) for CMSSW_7_6_X. Use std::lower_bound to add/search for userFloats/Ints/Cands/KinResolutions (76X) It involves the following packages: DataFormats/PatCandidates @cmsbuild, @vadler, @monttj can you please review it and eventually sign? Thanks. |
|
Do we care much about faster insert?
|
|
Fast read is guaranteed by this change. Was just trying to optimize as much as possible the writing too. At worst it is log + linear in the distance to end. Really not bad. |
|
Lookup here is always log. |
|
The only thing I'm not totally convinced is the change of the default for Giovanni On Thu, Oct 1, 2015 at 12:28 PM, Lindsey Gray [email protected]
|
|
@gpetruc thanks for the comment. I think giving wild (but calculable) values is better indicative of failure than returning zero, this way you force most cuts to fail (specifically with the choice of NaN for floats), or if storing indices to arrays in an object you'll likely go out of bounds. This means no quiet failures in analysis code, which I think is helpful. I agree that the change should be advertised, where should I be advertising this? This could be back ported into 74X just requiring an iorule to sort the names and stored values. Should I prepare this? (Perhaps 74X version leaves 0's for returned values when no label available.) |
|
Hi, Depending on how the thing is used, quiet failures may still happen, e.g. For 74X, I'd limit backports to fixes or features really needed for Giovanni On Thu, Oct 1, 2015 at 12:46 PM, Lindsey Gray [email protected]
|
|
Agreed. In that case I will update the PR so that we throw on missing ints/floats. |
|
@cmsbuild please test |
|
The tests are being triggered in jenkins. |
cmsbuild
added
tests-pending
comparison-pending
and removed
comparison-available
tests-approved
labels
Use std::lower_bound to add/search for userFloats/Ints/Cands/KinResolutions (76X)
Change PATObject so that it uses std::lower_bound to do the accounting of stored user information.
This should result in a very minor slow down in adding user data, and a huge speed up when retrieving it when there is a large number of user floats.
Also of note, previously if the same user Float/Int was added twice it would simply be appended to the end of the list. When retrieved a search from the front of the list was performed, so you'd only ever get the first value entered.
I've made it so that the original behavior is preserved but a complaint is generated if an overwrite is tried but not allowed (by boolean argument to the addUserFloat/Int() functions).
Also of note, the behavior of userFloat(key) and userInt(key) in the case that key does not exist has changed, both throw an exception if the key is not found.
This is a technical PR, no physics changes expected/observed in short matrix.