Optimize IB Index Lookup - #1916
Draft
danieljvickers wants to merge 1 commit into
Draft
danieljvickers wants to merge 1 commit into
danieljvickers wants to merge 1 commit into
Conversation
Lines of Code
|
|
Claude Code Review Head SHA: b080887 Files changed:
Findings:
|
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Contribution Policy
The IBM weak scaling runs performed previously achieved 64% scaling efficiency on all of Frontier, with the major slow down coming from the IB ownership handoff. A closer look at this subroutine reveals that the issue is a growing cost to rebuilt the IB lookup array as the number of IBs approaches 100s of millions of entries.
This PR optimizes that subroutine by refactoring the IB lookup array.
Old Lookup Behavior
The lookup was a sparce array of length
num_gbl_ibswhich held null values except for a few entries where global IBs were being tracked by the local neighborhood rank. At the largest simulation of 500 million IBs, this is about 2 GB of integers where only a few kilobytes hold required information.New Behavior
I have replaced the old lookup with a set of key-value-pair arrays that are sorted by key. Because we expect few IBs to be crossing boundaries, I opted to do an insertion sort where new IBs are inserted into the previously sorted array of elements. This reduces the size to track particles to the order of 100s of kilobytes. The lookup time slows slightly from O(1) to O(log(num_ibs)), but that time is still very negligible compared to the performance gain, which reduces the array rebuilding from O(num_gbl_ibs) to O(log(num_ibs)).
Before Merge
I am going to recheck the scaling on frontier and report a weak-scaling plot of the results before this is ready to be merged.