Cherry-pick bed-depth command from #259 - #263
Merged
Merged
Conversation
`depth` was renamed to `seg_depth`, and we needed to put `bedcmds` in the right place (under `ops`).
Just avoiding `*` in imports to make things more understandable.
It looks like code was duplicated across `bedcmds.rs` and `beddepth.rs`, so let's just keep done.
Includes allowing the `mut_range_bound` lint, which is known to have false positives for nested loops exactly like this one: rust-lang/rust-clippy#6072
Let's leave this in its own branch.
Instead of creating a file with a hard-coded name, we now just print the BED table to stdout. To put this in a file, use stream redirection, like `fgfa -I foo.gfa bed-depth 5 > foo.bed`.
Because this seems to be an end-to-end operation, let's avoid taking in a `Vec` parameter that we don't actually use.
Collapse the `windows` and `beddepth` modules into a single `window_depth` module, which is where we do everything relevant to this command. I have also renamed stuff to `window-depth` instead of `bed-depth` because I think this better describes what we're doing here, in terms of the end-to-end purpose. This also removes the "example" program I added in #252 because this fully supersedes it.
Try to explain how all this works, and simplify some of the Rust in the process.
You now pass the name of a path on the command line, and we produce that path's name in the BED output (which I think is how the odgi script works as well).
Turns out that odgi uses different digit counts for different commands. So we now can actually match odgi for this window-depth workload.
Collaborator
Author
|
I've done some commenting and some light organizations cleanup. The main interesting changes are in 2d7c87c, which @johnpalsberg may want to review. Here are the results from a little performance experiment (on my M1 Max MBP). I used this shell script for the odgi path: odgi depth -i chr8.pan.og -r chm13#chr8 | \
bedtools makewindows -b /dev/stdin -w 5000 > chm13.chr8.w5kbps.bed
odgi depth -i chr8.pan.og -b chm13.chr8.w5kbps.bed --threads 2 | \
bedtools sortAnd ran this hyperfine experiment: Here are the results: So 15x faster. Not bad! The outputs are pretty close but don't match precisely, for either FP precision or printing issues: |
This was referenced May 12, 2026
Closed
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.
Here is my attempt to take the parts from @johnpalsberg's #259 that add the new
bed-depthcommand and put them into a self-contained branch we can review, polish, and merge. Here are some loose ends to address before we finalize this:examples/added in All-Rust window depth example #252? That duplicatescompute_windows, for example.format_float) that we should be importing from existing modules?