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

Changes for speed improvements in clustering jobs #28

Merged
merged 2 commits into from
Sep 27, 2022
Merged

Changes for speed improvements in clustering jobs #28

merged 2 commits into from
Sep 27, 2022

Conversation

gartrog
Copy link
Contributor

@gartrog gartrog commented Jul 5, 2022

This PR should be merged at the same time as the one in k4FWCore.

Lots of small things that when put together allow to gain a lot.

  • CaloTopoClusters: use unordered_map instead of map
  • CaloTopoClusters: fix compilation warning
  • CaloTopoClusterInputTool: optimize use of map: update values instead
    of clearing and filling again
  • CaloTopoClusterInputTool: remove unused member variable
  • CaloTowerTool: update to use the new interface (do not fill map of
    links if not needed)
  • CaloTowerTool: micro-optimisation, reduce calls to eta and phi.
  • CreateCaloClustersSlidingWindow: use new interface of CaloTowerTool to
    not fill map of links if attachCells is not used
  • NoiseCaloCellsFromFileTool: use Decoder::get(ID, int) instead of string
  • NoiseCaloCellsFromFileTool: simplify finding bin in histogram
  • NoiseCaloCellsFromFileTool: micro-optimisation: avoid sqrt(x*x) when
    no pileup noise.
  • SplitClusters: fix dangerous compilation warning
  • CaloTopoClusterFCCee: use Decoder::get(ID, int) instead of string
  • CaloTopoClusterFCCee: to avoid too many lookups in the huge noise map,
    precompute minimum noise per layer and start by comparing noise to
    this minimum noise before looking into the big map.
  • CaloTopoClusterFCCee: micro-optimisation: avoid instantiating a
    vector<vector> of size 100000 at every call...
  • CaloTopoClusterFCCee: fix compilation warning
  • CreateCaloCellPositionsFCCee: create and use a cache of cell positions
    to avoid geomtry calls and conversions at every event
  • CreateCaloCellPositionsFCCee: use Decoder::get(ID, int) instead of string

Note: many alogirithmic optimisations are certainly still possible, I
focused on easy gains that did not require deep understanding of the
code.
I suspect many calls related to geometry are far from optimal.

- CaloTopoClusters: use unordered_map instead of map
- CaloTopoClusters: fix compilation warning
- CaloTopoClusterInputTool: optimize use of map: update values instead
  of clearing and filling again
- CaloTopoClusterInputTool: remove unused member variable
- CaloTowerTool: update to use the new interface (do not fill map of
  links if not needed)
- CaloTowerTool: micro-optimisation, reduce calls to eta and phi.
- CreateCaloClustersSlidingWindow: use new interface of CaloTowerTool to
  not fill map of links if attachCells is not used
- NoiseCaloCellsFromFileTool: use Decoder::get(ID, int) instead of string
- NoiseCaloCellsFromFileTool: simplify finding bin in histogram
- NoiseCaloCellsFromFileTool: micro-optimisation: avoid sqrt(x*x) when
  no pileup noise.
- SplitClusters: fix dangerous compilation warning
- CaloTopoClusterFCCee: use Decoder::get(ID, int) instead of string
- CaloTopoClusterFCCee: to avoid too many lookups in the huge noise map,
  precompute minimum noise per layer and start by comparing noise to
  this minimum noise before looking into the big map.
- CaloTopoClusterFCCee: micro-optimisation: avoid instantiating a
  vector<vector<pair>> of size 100000 at every call...
- CaloTopoClusterFCCee: fix compilation warning
- CreateCaloCellPositionsFCCee: create and use a cache of cell positions
  to avoid geomtry calls and conversions at every event
- CreateCaloCellPositionsFCCee: use Decoder::get(ID, int) instead of string

Note: many alogirithmic optimisations are certainly still possible, I
focused on easy gains that did not require deep understanding of the
code.
I suspect many calls related to geometry are far from optimal.
@gartrog
Copy link
Contributor Author

gartrog commented Jul 5, 2022

Tagging @vvolkl and @BrieucF

@BrieucF
Copy link
Contributor

BrieucF commented Aug 15, 2022

Just tested it and everything looks ok to me!

@BrieucF BrieucF merged commit 7c78a16 into HEP-FCC:main Sep 27, 2022
@faltovaj faltovaj mentioned this pull request Dec 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants