Remove deprecated RerankerCalculator and update references#4087
Remove deprecated RerankerCalculator and update references#4087michalkulakowski wants to merge 7 commits intoopenvinotoolkit:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR removes the deprecated MediaPipe RerankCalculator implementation and its associated proto/config artifacts, keeping the OpenVINO-based RerankCalculatorOV as the supported path.
Changes:
- Deleted the deprecated
RerankCalculatorC++ implementation and itsRerankCalculatorOptionsproto. - Removed Bazel targets and server build references to the deprecated calculator.
- Updated demos/test assets to stop exporting/whitelisting the deprecated calculator (but one test graph/config path still needs follow-up to stay consistent).
Reviewed changes
Copilot reviewed 9 out of 10 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
yarn.lock |
Adds missing lockfile entries for Bazel TypeScript deps (appears unrelated to rerank removal). |
src/test/rerank/with_params/invalid_graph_ov.pbtxt |
Removes legacy RerankCalculator node from the invalid OV graph (currently leaves the graph incomplete). |
src/test/rerank/with_params/invalid_graph.pbtxt |
Deletes legacy invalid graph using deprecated calculator. |
src/test/rerank/with_params/graph.pbtxt |
Deletes legacy graph using deprecated calculator. |
src/test/mediapipeflow_test.cpp |
Removes RerankCalculator from the registered-calculators whitelist test. |
src/rerank/rerank_calculator.proto |
Deletes deprecated calculator options proto. |
src/rerank/rerank_calculator.cc |
Deletes deprecated calculator implementation. |
src/rerank/BUILD |
Removes Bazel targets for deprecated calculator/proto. |
src/BUILD |
Removes deprecated calculator from server deps. |
demos/common/export_models/export_model.py |
Removes deprecated rerank export subcommand/templates, leaving rerank_ov. |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
| max_allowed_chunks: 4 | ||
| max_position_embeddings: 8 # invalid due to number of special tokens (4) + space for query (4) = 8, no space for document | ||
| } | ||
| calculator: "RerankCalculatorOV" |
There was a problem hiding this comment.
This file should look like src/test/rerank/graph_ov.pbtxt but with max_allowed_chunks and max_position_embeddings parameters
There was a problem hiding this comment.
Understood! I had blindly accepted the automated GitHub Copilot PR reviewer suggestion to fix the missing node ,I'll update this file to match your instructions
There was a problem hiding this comment.
Both OpenVINOModelServerSessionCalculator's should be removed since RerankCalculatorOV does not use them
| source-map-support "0.5.9" | ||
| tsutils "3.21.0" | ||
|
|
||
| "@bazel/worker@5.7.2": |
There was a problem hiding this comment.
@SaumyaTiwari108 please explain these changes
There was a problem hiding this comment.
Ah, My apology sir! My codespace environment accidentally auto-updated this file in the background and it slipped into the commit. I did not mean to change this. I will revert 'yarn.lock' back to the original main branch version right now.
There was a problem hiding this comment.
ok. please revert then
|
Sir @michalkulakowski, I want to contribute but seems like I'm not capable enough...can you please help me out to make me capable to contribute for your organisation if not this year(gsoc) then next year... |
|
@SaumyaTiwari108 You can run unit tests yourself and debug further. It looks like you forgot to remove unit tests from the source file. But please verify yourself as well :) |
No description provided.