Skip to content

refactor(umi): use binary search for count_index and deduplicate MoleculeId formatting - #146

Merged
nh13 merged 1 commit into
mainfrom
refactor/nh/simplify-fgumi-umi
Mar 3, 2026
Merged

nh13 merged 1 commit into
mainfrom
refactor/nh/simplify-fgumi-umi

Conversation

@nh13

@nh13 nh13 commented Mar 3, 2026

Copy link
Copy Markdown
Member

Summary

  • Replace O(n) linear .find() scan on count_index with O(log n) partition_point binary search in both build_adjacency_graph_bitenc and build_adjacency_graph — the array is sorted descending by count, making binary search straightforward
  • Simplify MoleculeId::to_string_with_offset to delegate to write_with_offset, removing duplicated format logic

Test plan

  • cargo nextest run -p fgumi-umi — all tests pass
  • cargo ci-fmt — clean
  • cargo ci-lint — clean

…culeId formatting

Replace linear scan of count_index in build_adjacency_graph with
partition_point binary search, reducing per-node lookup from O(n) to
O(log n) in the BFS inner loop.

Simplify MoleculeId::to_string_with_offset to delegate to
write_with_offset, removing duplicated format logic across three methods.
@nh13
nh13 temporarily deployed to github-actions March 3, 2026 03:26 — with GitHub Actions Inactive
@coderabbitai

coderabbitai Bot commented Mar 3, 2026 •

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3a460ea and fb4d570.

📒 Files selected for processing (2)
  • crates/fgumi-umi/src/assigner.rs
  • crates/fgumi-umi/src/lib.rs

📝 Walkthrough

Walkthrough

The changes optimize and refactor two modules in the fgumi-umi crate. In assigner.rs, a linear search for count constraints is replaced with a binary-search-based approach using partition_point, applied consistently across two mirrored code paths. A clippy expectation attribute is added for a complex section. In lib.rs, to_string_with_offset is refactored to reuse the existing write_with_offset function via a mutable buffer, eliminating inlined branching while preserving the public API and output behavior.

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the two main changes: binary search optimization for count_index and deduplication of MoleculeId formatting logic.
Description check ✅ Passed The description clearly explains both refactoring objectives, includes complexity improvements (O(n) to O(log n)), and documents the test plan covering all verification steps.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch refactor/nh/simplify-fgumi-umi

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@codecov

codecov Bot commented Mar 3, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 83.61%. Comparing base (3a460ea) to head (fb4d570).
⚠️ Report is 9 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #146   +/-   ##
=======================================
  Coverage   83.61%   83.61%           
=======================================
  Files         126      126           
  Lines       51510    51510           
=======================================
  Hits        43069    43069           
  Misses       8441     8441           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@nh13

nh13 commented Mar 3, 2026

Copy link
Copy Markdown
Member Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Mar 3, 2026

Copy link
Copy Markdown
✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@nh13

nh13 commented Mar 3, 2026

Copy link
Copy Markdown
Member Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Mar 3, 2026

Copy link
Copy Markdown
✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@nh13
nh13 merged commit 274d779 into main Mar 3, 2026
7 checks passed
@nh13
nh13 deleted the refactor/nh/simplify-fgumi-umi branch March 3, 2026 17:31
This was referenced Mar 3, 2026

This branch was previously deployed

1 inactive deployment
github-actions — fb4d570b Deployed Mar 3, 2026 by nh13 via coverage #510
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.

1 participant