Skip to content

Optimize IB Index Lookup - #1916

Draft
danieljvickers wants to merge 1 commit into
MFlowCode:masterfrom
danieljvickers:improve-ib-lookup-time
Draft

danieljvickers wants to merge 1 commit into
MFlowCode:masterfrom
danieljvickers:improve-ib-lookup-time

Conversation

@danieljvickers

Copy link
Copy Markdown
Member

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_ibs which 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.

@github-actions

Copy link
Copy Markdown

Lines of Code

File Lines Diff
src/simulation/m_ib_patches.fpp 618 +78
src/common/m_helper.fpp 506 +16
src/simulation/m_ibm.fpp 1321 -14
src/simulation/m_start_up.fpp 1238 -1
Directory Lines Diff
common 10437 +16
simulation 27911 +63
total 46759 +79

@github-actions

Copy link
Copy Markdown

Claude Code Review

Head SHA: b080887

Files changed:

  • 5
  • src/common/m_helper.fpp
  • src/simulation/m_collisions.fpp
  • src/simulation/m_ib_patches.fpp
  • src/simulation/m_ibm.fpp
  • src/simulation/m_start_up.fpp

Findings:

  • s_update_ib_lookup (src/simulation/m_ib_patches.fpp) is documented as "Completely rebuilds the ib lookup map, used at startup" and is exported publicly, but no call site is added anywhere in this diff. Since this is a brand-new subroutine (the old s_update_ib_lookup lived privately in m_ibm.fpp and is deleted here), any caller would have to appear as an added line in this diff, and none does — the two former call sites inside s_handoff_ib_ownership are replaced by s_compact_ib_lookup/s_merge_ib_lookup (incremental updates), not by a fresh rebuild. Meanwhile src/simulation/m_start_up.fpp (s_build_ib_neighborhood) simply deletes @:ALLOCATE(ib_gbl_idx_lookup(1:num_gbl_ibs)) without adding a replacement initialization call. s_compact_ib_lookup assumes ib_lookup_keys(1:num_ibs_old)/ib_lookup_vals(1:num_ibs_old) already hold a valid sorted mapping from the previous step, then does old_to_new(ib_lookup_vals(i)) — if the very first call to s_handoff_ib_ownership runs against locally-generated IB patches that were never seeded into ib_lookup_keys/ib_lookup_vals, those arrays contain undefined values, so ib_lookup_vals(i) can be an out-of-bounds/garbage index into old_to_new, corrupting the lookup table (or crashing) on the first timestep.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant