[4/5] Perf: build the flat fragment columns from the kept indices - #437
Open
GeorgWa wants to merge 1 commit into
Open
[4/5] Perf: build the flat fragment columns from the kept indices#437GeorgWa wants to merge 1 commit into
GeorgWa wants to merge 1 commit into
Conversation
Building the flat fragment dataframe materialised every column at the full dense length of n_fragment_rows * n_fragment_types, assembled them into a dataframe and then copied the retained subset out of it. Reannotating the precursor pointers allocated an int64 cumulative sum over all dense slots on top of that. Only mz and intensity are needed at dense length, because they give the keep mask. `_select_dense_fragments` accumulates that mask in place and returns the ascending indices of the kept slots. `_annotate_kept_fragments` then reads the type, the loss type and the charge straight off those indices, because the index of a slot gives its dense column. The dataframe is built from the kept values alone, so the filtering copy is gone. `_reannotate_precursor_pointers` finds a new pointer with a binary search on the kept indices, which is equivalent to subtracting the cumulative number of removed fragments. Output stays bit-identical to main across 5400 generated configurations of library shape, padding, top-k, intensity threshold and custom_columns: same fragment count, column order, dtypes, values and precursor pointers. At 48M dense slots, peak RSS drops from 3.49 GB to 1.77 GB and the call is ~27% faster. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
GeorgWa
changed the title
mschwoer
reviewed
Sep 1, 2026
| if "position" in custom_columns: | ||
| frag_df["position"] = positions.reshape(-1) | ||
|
|
||
| frag_df = {} |
| intensity, | ||
| n_fragment_types, | ||
| ) | ||
| del dense_directions |
Contributor
There was a problem hiding this comment.
did you check if this is garbage collected immediately? or does this need an explicit call to gc?
| del dense_directions | ||
|
|
||
| if intensity is None: | ||
| excluded = mz == 0 |
Contributor
There was a problem hiding this comment.
mz == 0 is shorthand for all array elements are zero? is there a more numpythonic way of doing that?
Comment on lines
+1082
to
+1083
| del is_padding | ||
| del not_top_k |
Contributor
There was a problem hiding this comment.
how large can the mz array be? is it worth the del noise?
| precursor_df["flat_frag_stop_idx"] -= cum_sum_tresh[ | ||
| precursor_df.flat_frag_stop_idx.values | ||
| ] | ||
| dense_start_idx = precursor_df.frag_start_idx.values.astype(np.int64) |
Contributor
There was a problem hiding this comment.
please use [] notation to access df columns consistently
| precursor_df.flat_frag_stop_idx.values | ||
| ] | ||
| dense_start_idx = precursor_df.frag_start_idx.values.astype(np.int64) | ||
| dense_stop_idx = precursor_df.frag_stop_idx.values.astype(np.int64) |
Contributor
There was a problem hiding this comment.
so frag_stop_idx is int32 (I assume), but needs to be int64 for the dense representation? worth a comment..
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
2 participants
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.
Performance. Output bit-identical to main.
flatten_fragmentsallocated every flat column at dense length (n_fragment_rows * n_fragment_types), assembled a dataframe, then copied the kept subset out of it. Pointer reannotation added anint64cumulative sum over all dense slots. For top-k libraries,mz == 0padding fills most dense slots.mzandintensityfrom the kept indices.np.searchsortedon the kept indices, instead of a dense cumulative sum.48M dense slots: peak RSS 3.49 GB -> 1.77 GB.
Stacked on #436. Splitting #429.
🤖 Generated with Claude Code