Skip to content

Consolidate parameter coercion and embed kwargs helpers into params/utils#1512

Merged
jdye64 merged 10 commits intoNVIDIA:mainfrom
jdye64:consolidate/pr5-param-wiring
Mar 9, 2026
Merged

Consolidate parameter coercion and embed kwargs helpers into params/utils#1512
jdye64 merged 10 commits intoNVIDIA:mainfrom
jdye64:consolidate/pr5-param-wiring

Conversation

@jdye64
Copy link
Collaborator

@jdye64 jdye64 commented Mar 7, 2026

Summary

Extract duplicated parameter coercion and embed-kwargs flattening logic from batch.py and inprocess.py into a shared params/utils.py module.

Both ingest modes contained identical _coerce_params functions and very similar blocks for flattening EmbedParams into a plain dict (merging runtime, optionally batch_tuning, and normalising embed_invoke_urlembedding_endpoint). This PR centralises that into two helpers:

  • coerce_params — merges an optional params object with override kwargs into a Pydantic model instance.
  • build_embed_kwargs — flattens an EmbedParams instance into a dict suitable for actor/task constructors, with an include_batch_tuning flag for the batch pipeline path.

Changes

  • New nemo_retriever/params/utils.pycoerce_params and build_embed_kwargs helpers.
  • New nemo_retriever/tests/test_params_utils.py — unit tests covering construction-from-kwargs, override application, embed_invoke_url normalisation, batch-tuning inclusion, and sub-model exclusion.
  • Simplified ingest_modes/batch.py — replaced inline _coerce_params and kwargs flattening block with imports from params.utils (~15 lines removed).
  • Simplified ingest_modes/inprocess.py — same replacement (~15 lines removed).

Stats

 4 files changed, 108 insertions(+), 30 deletions(-)

Test plan

  • New test_params_utils.py tests pass
  • Existing batch and inprocess tests still pass
  • pre-commit run --all-files passes (black, flake8)
  • Run batch pipeline end-to-end and verify embed stage behaves identically
  • Run inprocess pipeline end-to-end and verify embed stage behaves identically

Stack

This is PR 5 in the consolidation series:

  1. PR1 — embedder factory (merged)
  2. PR2 — LanceDB utils (merged)
  3. PR3 — recall helpers (consolidate/pr3-recall-helpers)
  4. PR4 — detection summary (consolidate/pr4-detection-summary)
  5. PR5 — param wiring (consolidate/pr5-param-wiring) ← this PR

Checklist

  • I am familiar with the Contributing Guidelines.
  • New or existing tests cover these changes.
  • The documentation is up to date with these changes.
  • If adjusting docker-compose.yaml environment variables have you ensured those are mimicked in the Helm values.yaml file.

jdye64 added 9 commits March 6, 2026 18:38
Centralises the resolve → branch → construct pattern for local HF embedding
models (VL and non-VL) that was duplicated across batch, inprocess, fused,
gpu_pool, recall, retriever, and text_embed code paths into a single
`create_local_embedder` factory function.

Made-with: Cursor
Extracts duplicated LanceDB row-building, schema definition, and
table-creation logic from batch.py and inprocess.py into a shared
ingest_modes/lancedb_utils.py module.

Made-with: Cursor
- Remove unused Path import and unused _extract_* aliases from inprocess.py
- Remove unused pytest import from test_lancedb_utils.py
- Apply black formatting to set literal and DataFrame constructor

Made-with: Cursor
…import

The ingest_modes __init__.py eagerly imports batch/fused/inprocess/online
which pull in ray, torch, etc. Pre-populate sys.modules with MagicMock
stubs so lancedb_utils tests can run in lightweight CI without those deps.

Made-with: Cursor
Centralises gold_to_doc_page, hit_key_and_distance, estimate_processed_pages,
and print_pages_per_second that were duplicated across batch, inprocess,
online, and fused pipeline examples. Fixes broken imports in fused_pipeline.py
that referenced non-existent functions in batch_pipeline.py.

Made-with: Cursor
Extracts duplicated detection summary computation and printing into a
shared utils/detection_summary.py module, replacing ~200 lines of
near-identical logic in batch_pipeline.py and inprocess.py with thin
wrappers around the shared implementation.

Made-with: Cursor
Consolidates the duplicated _coerce_params pattern and embed parameter
flattening logic from batch.py and inprocess.py into a shared
params/utils.py module with coerce_params and build_embed_kwargs helpers.

Made-with: Cursor
@jdye64 jdye64 marked this pull request as ready for review March 9, 2026 15:50
@jdye64 jdye64 requested a review from a team as a code owner March 9, 2026 15:50
@jdye64 jdye64 requested a review from edknv March 9, 2026 15:50
@jdye64 jdye64 merged commit 4268253 into NVIDIA:main Mar 9, 2026
8 checks passed
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.

2 participants