enrico-dev / enrico

ENRICO: Exascale Nuclear Reactor Investigative COde
https://enrico-docs.readthedocs.io
BSD 3-Clause "New" or "Revised" License
62 stars 25 forks source link

Improve scaling of OpenmcDriver::cell_index #151

Closed paulromano closed 2 years ago

paulromano commented 2 years ago

We recently ran into a scaling issue when running coupled NekRS + OpenMC runs with very large models. After some investigation, it was determined that the following lines were causing the issue: https://github.com/enrico-dev/enrico/blob/91a4f702d34c6deb61db4edd472d2b5f86e14a54/src/coupled_driver.cpp#L372-L375

In particular, when the neutronics driver was OpenmcDriver, we had the following implementation of cell_index: https://github.com/enrico-dev/enrico/blob/91a4f702d34c6deb61db4edd472d2b5f86e14a54/src/openmc_driver.cpp#L195-L200

Lines 197 and 199 are both potentially problematic. cells_ being an ordered map, has a find which is O(log N) on average. Even worse is the std::distance which is O(N). Together, we were finding that on a full core SMR model, the for loop above was taking 200 seconds for runs on about 200 nodes of Summit.

My solution here is to replace our original std::map<CellHandle, CellInstance> cells_ with std::vector<CellInstance> cells_ along with std::unordered_map<CellHandle, index>, where the index refers to the index in cells_. This allows us to do O(1) determination of cell instances and indices (i.e., cell_index). @RonRahaman has already observed a reduction in time for the heat source update of about two orders of magnitude on a single node using these changes.

RonRahaman commented 2 years ago

This is great, thanks for getting to the bottom of this!