Skip to content

Add joint/centroid embedding and concept-to-concept similarity queries#40

Open
nicoloesch wants to merge 6 commits into
mainfrom
31-joint-similarity-queries
Open

Add joint/centroid embedding and concept-to-concept similarity queries#40
nicoloesch wants to merge 6 commits into
mainfrom
31-joint-similarity-queries

Conversation

@nicoloesch

Copy link
Copy Markdown
Collaborator

Summary

Adds two composition methods to EmbeddingReaderInterface:

  • get_joint_embedding(concept_ids, weights=None) (weighted-mean centroid, normalized for cosine space), and
  • get_similar_concepts(concept_id, k, *, concept_filter=None) (single-call nearest-neighbor lookup, built on the former).

Both are pure compositions of existing get_embeddings_by_concept_ids/get_nearest_concepts. No backend or schema changes.

@nicoloesch nicoloesch assigned gkennos and unassigned gkennos Jul 10, 2026
@nicoloesch
nicoloesch requested a review from gkennos July 10, 2026 03:25
@nicoloesch

Copy link
Copy Markdown
Collaborator Author

Lint PR is failing because it is not adhering to our current CI/CD. Related to #39 and the decisions there.

@gkennos gkennos left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

also in docs/usage/interface-guide.md add docs for the two new functions

Comment thread src/omop_emb/interface.py
Comment thread src/omop_emb/interface.py
@nicoloesch

Copy link
Copy Markdown
Collaborator Author

Accepted and implemented all review comments. I addressed the obfuscation of k and limit using another data structure that mirrors what is already in omop_graph and tracked here: AustralianCancerDataNetwork/OMOP_Alchemy#11
Folr more information, look at the response to the comment.

Comment thread docs/usage/interface-guide.md
@nicoloesch nicoloesch added the breaking Incompatible API change. MAJOR: x+1.y.z label Jul 20, 2026
@nicoloesch

nicoloesch commented Jul 21, 2026

Copy link
Copy Markdown
Collaborator Author

Update: reverted the EmbeddingConceptFilter/CDMConceptFilter type split from this PR (commits d39a723, c7cb1b9).

That split fixed the k/limit precedence issue correctly, but it's a breaking change to a released public type: EmbeddingConceptFilter.limit is used directly by omop-graph in 3 places, and removing it isn't worth a v2.0 bump on its own right now.

Replaced it with a backward-compatible fix instead: the underlying SQL-level bug (backends overriding k with concept_filter.limit) is still fixed, and EmbeddingReaderInterface now emits a DeprecationWarning whenever concept_filter.limit is used for KNN search, raising ValueError if it's given alongside a conflicting explicit k. EmbeddingConceptFilter itself is untouched. No downstream consumer needs to change anything for this release.

The real type split is tracked as a separate, future breaking-change issue: #48.

Label: breaking back to feature.

@nicoloesch nicoloesch added feature New backwards-compatible functionality. MINOR: x.y+1.z and removed breaking Incompatible API change. MAJOR: x+1.y.z labels Jul 21, 2026
@nicoloesch
nicoloesch requested a review from gkennos July 21, 2026 04:14
Comment thread src/omop_emb/interface.py

@gkennos gkennos left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

still breaking - don't think there are active consumers, honestly, but since we are actively pursuing improved release hygiene, it seems worth retaining for now

@nicoloesch
nicoloesch requested a review from gkennos July 23, 2026 04:04
@nicoloesch

Copy link
Copy Markdown
Collaborator Author

Decided to restore the function and add the DeprecationWarning so we can remove it in 2.0. It should now just be a feature.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature New backwards-compatible functionality. MINOR: x.y+1.z

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add single-call concept-to-concept similarity query Add joint/centroid embedding query to EmbeddingReaderInterface

2 participants