Skip to content

MaveDB: only keep the columns and records that can be used - #853

Open
davmlaw wants to merge 1 commit into
Ensembl:mainfrom
davmlaw:mavedb-reduce-memory
Open

MaveDB: only keep the columns and records that can be used#853
davmlaw wants to merge 1 commit into
Ensembl:mainfrom
davmlaw:mavedb-reduce-memory

Conversation

@davmlaw

@davmlaw davmlaw commented Aug 12, 2026

Copy link
Copy Markdown

Fixes the memory usage reported in #852, where a single 229 bp deletion makes the plugin allocate
9.7 GB

Two independent changes, both bounded by the variant's reference span rather than by anything the user
controls:

Keep only the columns parse_data needs. It built %res keyed on every column in the data file
(259 in the current distribution) for every record in the span, and _join_results narrowed to cols
afterwards — so cols never reduced the memory. _set_parsed_columns() works the wanted indexes out
once in new(). It keeps MaveDB_hgvs and MaveDB_refseq beyond cols, since run() reads those
directly, and only when the option that consults them is enabled.

Skip records that cannot match. Score sets mapped through VRS carry a ga4gh:VA.<digest> in ref
instead of sequence — 77% of records in the current file. get_matched_variant_alleles compares alleles
as strings, so those can never match; they were parsed, hashed and held anyway.

On the 229 bp deletion from #852, against the current MaveDB_variants.tsv.gz:

peak RSS wall
before 9.68 GB 19.7 s
columns only 0.71 GB 9.7 s
record skip only 0.08 GB 3.6 s
this PR 0.08 GB 3.6 s

0.08 GB is what the same run costs with the plugin not loaded at all.

Output is unchanged

Compared line by line against the current plugin, identical in every case:

  • t/testdata/cache (chr21, GRCh38) with the chr21 slice of the MaveDB file, over 352 variants from
    t/testdata/input/test.vcf plus records inside the cache window — 2,112 MaveDB values from both,
    0.20 GB → 0.10 GB.
  • 824 variants sampled genome-wide from the MaveDB file — 31,441 MaveDB values from both,
    0.52 GB → 0.35 GB.
  • Every combination of single_aminoacid_changes / transcript_match (0/0, 0/1, 1/0, 1/1), and
    cols=all. cols=all is the useful case: it makes the projection a no-op and exercises the record
    skip on its own.

Targeting main per CONTRIBUTING.md.

parse_data built a hash keyed on every column in the data file for every
record in a variant's span, regardless of what cols asked for, and kept
records whose ref is a ga4gh:VA identifier rather than sequence - which
get_matched_variant_alleles can never match.

Both are unbounded by variant size but bounded by reference span, so a
long deletion over a deep mutational scanning locus pays for them
hundreds of thousands of times over: a 229 bp deletion in UBC peaks at
9.7 GB and produces no annotation.

Keep only the columns parse_data needs - those in cols, plus MaveDB_hgvs
and MaveDB_refseq where the option that reads them is enabled - and skip
records that cannot match. Same deletion now peaks at 0.08 GB, matching
a run with the plugin not loaded, with output unchanged.

Ensembl#852
@davmlaw

davmlaw commented Aug 12, 2026

Copy link
Copy Markdown
Author

One clarification on the table in the description, since it invites the wrong conclusion: on that 229 bp
deletion the record skip alone already reaches the floor, which makes the column projection look
redundant. It isn't — that variant is at one extreme, where 100% of the records fetched are VRS-mapped.

Measured on VEP's own shipped test data (t/testdata/cache, chr21) there is a case at the other
extreme: a 1 kb deletion whose span contains no VRS-mapped records at all, where the skip does nothing
and the projection provides the entire saving (0.13 GB -> 0.08 GB).

Full numbers and a reproduction recipe: #852 (comment)
#852 (comment)

@ainefairbrother
ainefairbrother self-requested a review August 13, 2026 10:06

@ainefairbrother ainefairbrother left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @davmlaw,

Thank you for this fix. I have tested it and see that, indeed, it does speed up the plugin.

@davmlaw

davmlaw commented Aug 14, 2026

Copy link
Copy Markdown
Author

Cool. It was mostly the RAM use for me (I'm running a few VEPs in parallel on a VM) but faster is good too.

I think the Mave plugin was fine for the initial version of the data, but MAVE has grown a huge amount over the last few years (which is great!) just need a few adjustments to keep up

@nakib103
nakib103 self-requested a review August 14, 2026 08:21
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.

3 participants