SACGF / SACGF/variantgrid

Use MemoryOntologyTraverser for batch condition matching

Open
#1,549 3 comments 0 reactions 1 assignee Claimed by @TheMadBug View on GitHub
optimisation
Dominant language
Python
Stars
30
Forks
3
Avg merge
9h 28m
Merged PRs (30d)
42

Description

🤖 Written by Claude

Follow-up to the gene_annotation in-memory traverser work.

## Background

`gene_annotation` now constructs a `MemoryOntologyTraverser` once per batch and shares it across the ~135 k `terms_for_gene_symbol` calls it issues. The condition-matching subsystem has the same shape — long-running operations that issue many `OntologySnake.*` calls per gene-symbol/term — but still goes call-by-call against the live DB.

## Benchmark (local DB, OntologyVersion v1, 265,689 OntologyTermRelations, 45,652 HGNC terms)

| | time |
|---|---|
| `MemoryOntologyTraverser` build | **12.3 s** (one-shot) |
| `DbOntologyTraverser` build | ~0 ms |
| DB per `terms_for_gene_symbol` call | **9.74 ms** |
| Memory per `terms_for_gene_symbol` call | **0.01 ms** (~1000× faster) |

200 random HGNC × 3 services (OMIM/HPO/MONDO) = 600 calls per traverser. Note: only ~5,272 of 45,652 HGNC terms have any relations, so the random sample is dominated by empty lookups — DB per-call cost rises (and crossover comes down) for batches concentrated on symbols with relations.

**Crossover: ~1,260 calls, or ~420 genes when calling 3 services per gene.** Above that, the build cost is amortised.

## Candidate batch boundaries

| Site | Shape | Likely scale |
|---|---|---|
| `ConditionTextMatch.sync_all()` (`classification/models/condition_text_matching.py:244`) | iterates every published `ClassificationModification`, then every `ConditionText`; `attempt_automatch` → `is_auto_assignable` → `OntologySnake.has_gene_relationship` per gene-level | thousands of calls per run |
| Sync/import auto-match (`condition_text_matching.py:658,670`) | same `sync_condition_text_classification(..., attempt_automatch=True)` path during Shariant/Alissa imports | depends on import size |
| `condition_matching_report` command (`classification/management/commands/condition_matching_report.py:88`) | one `OntologySnake.snake_from(term, HGNC)` per unique `ConditionTextMatch.ontology_term` | hundreds–low thousands |

## API gap

The traverser interface (`ontology/ontology_traversal.py`) only exposes `snake_from`, `terms_for_gene_symbol`, `gene_disease_relations`. Condition matching uses two more `OntologySnake` static methods that aren't on the traverser:

- `OntologySnake.has_gene_relationship(term, gene_symbol, quality_filter)` — `models_ontology.py:1159`
- `OntologySnake.get_all_term_to_gene_relationships(term, gene_symbol, try_related_terms)` — `models_ontology.py:1191`

Neither accepts an `otr_qs` argument today; both call `OntologyVersion.get_latest_and_live_ontology_qs()` directly. They need to grow the kwarg before the DB traverser can plumb it, and the memory traverser needs in-memory equivalents.

## Sketch of changes

### 1. Extend traverser interface

```python
# ontology/ontology_traversal.py

class OntologyTraverser(Protocol):
def snake_from(...): ...
def terms_for_gene_symbol(...): ...
def gene_disease_relations(...): ...
def has_gene_relationship(self, term: OntologyTerm | str,
gene_symbol: GeneSymbol | str,
quality_filter: OntologyRelationshipQualityFilter = ONTOLOGY_RELATIONSHIP_STANDARD_QUALITY_FILTER) -> bool: ...
def get_all_term_to_gene_relationships(self, term: OntologyTerm | str,
gene_symbol: GeneSymbol | str,
try_related_terms: bool = True) -> Iterator['OntologySnake']: ...
```

`DbOntologyTraverser` delegates to the existing static methods (after they grow `otr_qs=`). `MemoryOntologyTraverser` re-implements both against the adjacency dicts — `has_gene_relationship` is a single-hop dict lookup with the MONDO/OMIM equivalence dance; `get_all_term_to_gene_relationships` enumerates outgoing edges from `term` to the HGNC term plus the MONDO↔OMIM bridge.

### 2. Add `otr_qs=` to the static helpers

```python
# ontology/models/models_ontology.py

@staticmethod
def has_gene_relationship(term, gene_symbol, quality_filter=...,
otr_qs: QuerySet[OntologyTermRelation] = None,
call_update_gene_relations: bool = True) -> bool:
if call_update_gene_relations:
from ontology.panel_app_ontology import update_gene_relations
update_gene_relations(gene_symbol)
if otr_qs is None:
otr_qs = OntologyVersion.get_latest_and_live_ontology_qs()
...

@staticmethod
def get_all_term_to_gene_relationships(term, gene_symbol, try_related_terms=True,
otr_qs: QuerySet[OntologyTermRelation] = None,
call_update_gene_relations: bool = True):
...
```

### 3. Plumb traverser through condition matching

`attempt_automatch` → `is_auto_assignable` → `has_gene_relationship` is the chain. Threading a traverser explicitly is verbose; cleaner is a context-managed default the leaf falls back to:

```python
# ontology/ontology_traversal.py

_active_traverser: ContextVar[OntologyTraverser | None] = ContextVar(
"active_traverser", default=None)

@contextmanager
def use_traverser(traverser: OntologyTraverser):
token = _active_traverser.set(traverser)
try:
yield traverser
finally:
_active_traverser.reset(token)

def get_active_traverser() -> OntologyTraverser | None:
return _active_traverser.get()
```

Leaf calls in `OntologySnake.has_gene_relationship` / `get_all_term_to_gene_relationships` route through the active traverser if one is set, otherwise fall back to today's behaviour. Batch boundary becomes:

```python
# classification/models/condition_text_matching.py

@staticmethod
def sync_all():
ov = OntologyVersion.latest()
with use_traverser(MemoryOntologyTraverser(ov)):
cms = ClassificationModification.objects.filter(...)
for cm in cms:
ConditionTextMatch.sync_condition_text_classification(cm=cm, update_counts=False)
for ct in ConditionText.objects.all():
ConditionTextMatch.attempt_automatch(condition_text=ct)
...
```

Same pattern wraps the bulk Shariant/Alissa import entry point. `condition_matching_report.Command.handle` builds its own and uses `traverser.snake_from(term, HGNC)` directly.

### 4. Bulk-fetch PanelApp once at the batch boundary

Reuse `bulk_update_gene_relations()` (in `ontology/panel_app_ontology.py`) so the in-loop hook can be skipped — same shape as `gene_annotation`'s `_bulk_fetch_panel_app`:

```python
from ontology.panel_app_ontology import bulk_update_gene_relations

bulk_update_gene_relations() # one paginated crawl of /api/v1/genes/
with use_traverser(MemoryOntologyTraverser(ov)):
...
```

`bulk_update_gene_relations()` ignores `settings.GENE_RELATION_PANEL_APP_LIVE_UPDATE` — calling it is the opt-in. Pair with `call_update_gene_relations=False` on the traverser to short-circuit the per-call `update_gene_relations(gene_symbol)` hook (cheap-but-pointless after the bulk crawl).

## When this is worth doing

- **`sync_all()`**: almost certainly above 420 genes — Shariant has thousands of classifications across hundreds–thousands of distinct ConditionTexts. Strong candidate.
- **Shariant/Alissa imports**: depends on import size. A single classification import isn't worth it; a full re-sync is.
- **`condition_matching_report`**: marginal — depends on per-lab volume. Still cleanest to use the traverser API (DB mode) for consistency, even if memory mode isn't always picked.

Memory footprint of the snapshot (~265 k edge objects + their `OntologyTerm` foreign objects) hasn't been measured; should be checked before defaulting in-memory mode on for any of these.

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.