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

combineTPGBT: Attack of the Clones (aka a complete overhaul of combineTPGBT) #26

Merged
merged 7 commits into from Aug 14, 2017

Conversation

Projects
None yet
2 participants
@jrfarah
Copy link
Contributor

jrfarah commented Aug 11, 2017

Previously, combineTPGBT iterated through the GBT dataset and looked for matches in TPFit. When one was found, it would add all the TPfit data to a new tree.

The new and improved combineTPGBT changes many things:

  • Creates clone of TPfit instead of cherry picking and combining all data, then adds the necessary GBT data to the tree
  • Only one fill is featured in the code, which simplifies logic and protects against accidental duplicates being added to the combined output.
  • Board IP assignment is now a separate function, which improves readability and makes it easier to maintain in the future.
  • Most, if not all, FIT operations have been removed from the GBT loop, which prevents unnecessary repetition of operations
  • Comments have been improved; pointless ones have been removed and more relevant ones have been added.

Some smaller things:

  • The code has been cleaned up and made prettier!
  • A new type has been defined to improve the readability of the code. std::vector<int> is now int_v.
  • A bunch of unnecessary data hand-offs were removed, freeing up memory, shortening compile times, and reducing run times.
  • All compiler warnings have been fixed. Less stress all-around.
  • All instances of the reserve() function have been removed, shortening compile times. Also, they weren't really doing anything.

Some timing data:

  • Python implementation: ~78-240 minutes for whole dataset

  • C++ implementation: ~4.7 minutes for whole dataset (1700% time increase)

  • Python implementation: ~500 Hz average

  • C++ implementation: ~30,000 Hz average (depending on whether or not computer is on AC power) (6000% speed increase)

  • Why is the speed increase greater than the time increase?

    • This is largely due to data corruption causing slowdowns that result in the elapsed time deviating from the time predicted by the operation-per-second count. The changes can be seen more accurately if the code is run over a smaller (say, 100,000 iterations) subset of the full dataset.

Thanks to @alexandertuna and @sezata for constant guidance and feedback on the code.

@sezata sezata merged commit 9fa999e into crogan:master Aug 14, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.