Skip to content

fix(cross_ontology_search): embed cache, strict cosine, early qv=None return, hardware comment#561

Merged
turbomam merged 4 commits into
mainfrom
copilot/fix-typo-in-gpu-designation
Jun 29, 2026
Merged

fix(cross_ontology_search): embed cache, strict cosine, early qv=None return, hardware comment#561
turbomam merged 4 commits into
mainfrom
copilot/fix-typo-in-gpu-designation

Conversation

Copilot AI commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

Four independent improvements to metpo/pipeline/cross_ontology_search.py addressing correctness, performance, and clarity.

Changes

  • Comment — replace "M5 GPU" with "CPU/GPU-capable hardware"; the original reference was ambiguous/non-standard

  • Embedding cache — add module-level _EMBED_CACHE and _get_cached_embedding() to deduplicate Ollama API calls within a run; previously the same text (e.g. a label shared across multiple terms) would trigger a fresh HTTP round-trip each time

    _EMBED_CACHE = {}
    
    def _get_cached_embedding(text, model, embed_url):
        key = (model, embed_url, text.strip(". "))
        if key not in _EMBED_CACHE:
            _EMBED_CACHE[key] = embed(key[2], model, embed_url)
        return _EMBED_CACHE[key]
  • Strict cosine() — explicit dimension-mismatch check raises ValueError before the dot product; zip() upgraded to strict=True as a belt-and-suspenders guard; mismatched vectors were previously silently truncated

  • Early return on qv = None in rank_candidates()** — when the query embedding fails, fall back immediately to lexical ranking instead of looping over all candidates, calling embed() for each, and assigning misleading 0.0 scores; also drops the now-redundant qv and from the per-candidate guard

Original prompt
Please apply the following diffs and create a pull request.
Once the PR is ready, give it a title based on the messages of the fixes being applied.

[{"message":"The comment mentions 'M5 GPU' which appears to be a typo or non-standard GPU designation. Common GPU models include M1/M2/M3 (Apple Silicon) or NVIDIA/AMD models. Consider clarifying the intended GPU type or removing the specific hardware reference if it's meant to be generic.","fixFiles":[{"filePath":"metpo/pipeline/cross_ontology_search.py","diff":"diff --git a/metpo/pipeline/cross_ontology_search.py b/metpo/pipeline/cross_ontology_search.py\n--- a/metpo/pipeline/cross_ontology_search.py\n+++ b/metpo/pipeline/cross_ontology_search.py\n@@ -36,7 +36,7 @@\n import click\n \n OLS_SEARCH = \"https://www.ebi.ac.uk/ols4/api/search\"\n-# Default embedding backend: free, local, runs on the M5 GPU. Nothing paid, no API\n+# Default embedding backend: free, local, runs on CPU/GPU-capable hardware. Nothing paid, no API\n # key. Both the endpoint and the model are CLI-parameterized so anyone can point at\n # a different local model, a remote Ollama server, or other hardware.\n DEFAULT_EMBED_URL = \"http://localhost:11434/api/embeddings\"\n"}]},{"message":"The `strict=False` parameter in `zip()` silently truncates if vectors have different lengths, which could mask data quality issues. For cosine similarity calculations, vectors should have matching dimensions. Consider using `strict=True` to catch dimension mismatches early, or add explicit length validation before the calculation.","fixFiles":[{"filePath":"metpo/pipeline/cross_ontology_search.py","diff":"diff --git a/metpo/pipeline/cross_ontology_search.py b/metpo/pipeline/cross_ontology_search.py\n--- a/metpo/pipeline/cross_ontology_search.py\n+++ b/metpo/pipeline/cross_ontology_search.py\n@@ -91,7 +91,10 @@\n \n \n def cosine(a, b):\n-    dot = sum(x * y for x, y in zip(a, b, strict=False))\n+    if len(a) != len(b):\n+        msg = f\"cosine vector dimension mismatch: len(a)={len(a)} != len(b)={len(b)}\"\n+        raise ValueError(msg)\n+    dot = sum(x * y for x, y in zip(a, b, strict=True))\n     na = math.sqrt(sum(x * x for x in a))\n     nb = math.sqrt(sum(x * x for x in b))\n     return dot / (na * nb) if na and nb else 0.0\n"}]},{"message":"The function makes N+1 embedding API calls (1 for query + N for candidates), which can be slow for large candidate sets. Consider batching candidate embeddings if the Ollama API supports batch requests, or caching embeddings for frequently-queried candidates to reduce redundant API calls.","fixFiles":[{"filePath":"metpo/pipeline/cross_ontology_search.py","diff":"diff --git a/metpo/pipeline/cross_ontology_search.py b/metpo/pipeline/cross_ontology_search.py\n--- a/metpo/pipeline/cross_ontology_search.py\n+++ b/metpo/pipeline/cross_ontology_search.py\n@@ -42,7 +42,10 @@\n DEFAULT_EMBED_URL = \"http://localhost:11434/api/embeddings\"\n DEFAULT_EMBED_MODEL = \"nomic-embed-text\"\n \n+# In-memory cache to avoid redundant embedding API calls within a run.\n+_EMBED_CACHE = {}\n \n+\n def ols_candidates(label, rows=20):\n     \"\"\"Lexical candidate classes from OLS4 for a label (IRI, curie, label, def, ontology).\"\"\"\n     q = urllib.parse.urlencode({\"q\": label, \"type\": \"class\", \"rows\": rows})\n@@ -90,6 +92,14 @@\n         return None\n \n \n+def _get_cached_embedding(text, model, embed_url):\n+    normalized = text.strip(\". \")\n+    key = (model, embed_url, normalized)\n+    if key not in _EMBED_CACHE:\n+        _EMBED_CACHE[key] = embed(normalized, model, embed_url)\n+    return _EMBED_CACHE[key]\n+\n+\n def cosine(a, b):\n     dot = sum(x * y for x, y in zip(a, b, strict=False))\n     na = math.sqrt(sum(x * x for x in a))\n@@ -130,9 +140,9 @@\n def rank_candidates(term, cands, use_embeddings, model, embed_url):\n     \"\"\"Attach a 'similarity' score to each candidate (embedding cosine or lexical order).\"\"\"\n     if use_embeddings and cands:\n-        qv = embed((term[\"label\"] + \". \" + term[\"definition\"]).strip(\". \"), model, embed_url)\n+        qv = _get_cached_embedding(term[\"label\"] + \". \" + term[\"definition\"], model, embed_url)\n         for c in cands:\n-            cv = embed((c[\"label\"] + \". \" + c[\"definition\"]).strip(\". \"), model, embed_url)\n+            cv = _get_cached_embedding(c[\"label\"] + \". \" + c[\"definition\"], model, embed_url)\n             c[\"similarity\"] = round(cosine(qv, cv), 4) if (qv and cv) else 0.0\n         cands.sort(key=lambda c: c[\"similarity\"], reverse=True)\n     else:\n"}]},{"message":"If `qv` (query vector) is None, the function continues to embed all candidates but assigns similarity=0.0 to all. This wastes API calls and produces misleading zero-similarity scores. Consider returning early or skipping embedding calls when qv is None to avoid unnecessary work and confusing output.","fixFiles":[{"filePath":"metpo/pipeline/cross_ontology_search.py","diff":"diff --git a/metpo/pi...

Copilot AI changed the title [WIP] Fix typo in GPU designation comment fix(cross_ontology_search): embed cache, strict cosine, early qv=None return, hardware comment Jun 29, 2026
Copilot AI requested a review from turbomam June 29, 2026 14:50
@turbomam turbomam marked this pull request as ready for review June 29, 2026 14:52
Copilot AI review requested due to automatic review settings June 29, 2026 14:52

Copilot AI 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.

Pull request overview

This PR improves metpo/pipeline/cross_ontology_search.py by reducing redundant embedding calls, tightening cosine similarity correctness, and improving behavior when embedding is unavailable, while also clarifying a hardware-related comment.

Changes:

  • Add an in-process embedding cache to deduplicate repeated Ollama /api/embeddings requests within a run.
  • Make cosine() strict about vector dimension mismatches (explicit check + zip(..., strict=True)).
  • If the query embedding fails (qv is None), immediately fall back to lexical ranking instead of embedding every candidate and assigning misleading 0.0 similarities.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread metpo/pipeline/cross_ontology_search.py
turbomam and others added 2 commits June 29, 2026 10:57
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@turbomam turbomam merged commit d974acc into main Jun 29, 2026
15 checks passed
@turbomam turbomam deleted the copilot/fix-typo-in-gpu-designation branch June 29, 2026 15:02
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