Add Interleaved filters - #1583
Conversation
Greptile SummaryThis PR adds four interleaved dataset filters — blur detection, QR code coverage, CLIP image-text score, and image-to-text ratio — implemented as stateless, vectorised Prior review concerns (redundant per-image text-embedding forward passes, the Confidence Score: 5/5Safe to merge; all remaining findings are P2 style suggestions that do not affect correctness. Prior P1 concerns (redundant text-embedding forward passes, multiprocessing deadlock, indices/images list divergence) have all been addressed. The two new findings are both P2: the RGB-vs-grayscale Laplacian choice and a missing column-existence guard that is practically unreachable given the schema invariant. No blocking issues remain. blur_filter.py (RGB Laplacian threshold calibration), image_utils.py (None-return path from imdecode discussed in prior threads)
|
| Filename | Overview |
|---|---|
| nemo_curator/stages/interleaved/filter/blur_filter.py | Sharpness scored via Laplacian variance on the full RGB image rather than grayscale — numerically valid but slightly non-standard. |
| nemo_curator/stages/interleaved/filter/clip_score_filter.py | CLIP filter now groups images by sample_id and calls encode_text once per sample; indices/images alignment is correct in _indices_and_decoded_images_from_rows. |
| nemo_curator/stages/interleaved/filter/qrcode_filter.py | QR filter now uses detectAndDecodeMulti retval+points check exclusively; previous fallback issue removed. |
| nemo_curator/stages/interleaved/filter/image_to_text_ratio_filter.py | Ratio computed vectorially via groupby/map; boundary-inclusive comparison and fillna passthrough are correct. |
| nemo_curator/stages/interleaved/utils/image_utils.py | image_bytes_to_array wraps both imdecode and cvtColor in a single cv2.error handler; the None-return path from imdecode was discussed in prior review threads. |
| nemo_curator/models/clip.py | CLIPImageEmbeddings gains an encode_text method; both image and text embeddings are L2-normalised before the dot-product score in the filter. |
| benchmarking/scripts/interleaved_filter_benchmark.py | New benchmark wires all four filter stages into a pipeline with configurable thresholds; YAML entries are disabled by default. |
Sequence Diagram
sequenceDiagram
participant Batch as InterleavedBatch
participant Blur as BlurFilterStage
participant QR as QRCodeFilterStage
participant CLIP as CLIPScoreFilterStage
participant Ratio as ImageToTextRatioFilterStage
Batch->>Blur: content_keep_mask(df)
Note over Blur: iter image rows<br/>cv2.Laplacian variance >= threshold?
Blur-->>Batch: keep_mask (image rows filtered)
Batch->>QR: content_keep_mask(df)
Note over QR: iter image rows<br/>QR area / img_area < threshold?
QR-->>Batch: keep_mask (image rows filtered)
Batch->>CLIP: content_keep_mask(df)
Note over CLIP: group images by sample_id<br/>encode_text(texts) once per sample<br/>img_emb @ text_emb.T -> max score >= min_score?
CLIP-->>Batch: keep_mask (image rows filtered)
Batch->>Ratio: content_keep_mask(df)
Note over Ratio: groupby sample_id<br/>image_count / max(word_count, 1)<br/>min_ratio <= ratio <= max_ratio?
Ratio-->>Batch: keep_mask (all rows in sample filtered)
Reviews (12): Last reviewed commit: "Merge branch 'main' into bobchen/interle..." | Re-trigger Greptile
VibhuJawa
left a comment
There was a problem hiding this comment.
Reviewed 2 filters, requested changes.
Would recommend following patterns in text modality here for model forward pass of batching etc and not using multi processing with in a task
| idx, image_bytes = item | ||
| if image_bytes is None: | ||
| return (idx, False) | ||
| image = _image_bytes_to_array(image_bytes) |
There was a problem hiding this comment.
This means that we always decode bytes to numpy arrays . We should add a buffer to the structure and reuse where possible
There was a problem hiding this comment.
I think we should create another PR for this. It requires changes on InterleavedBatch
There was a problem hiding this comment.
Okay, please file an issue please so that we can track it . We can do this in the next release but we should track the issue.
d1fc285 to
a2f7285
Compare
317e2be to
6b9a3ec
Compare
VibhuJawa
left a comment
There was a problem hiding this comment.
Thanks for the work here, please also add benchmarking scripts as part of this PR .
Signed-off-by: meatybobby <meatybobby@gmail.com>
2bc7981 to
79467a7
Compare
|
/ok to test da859f3 |
VibhuJawa
left a comment
There was a problem hiding this comment.
I think a bunch of earlier reviews are still not addressed. Please address them
|
@claude review |
|
/ok to test f2e5f1c |
|
/ok to test b9caafc |
VibhuJawa
left a comment
There was a problem hiding this comment.
Thanks for the addressing feedback. The only thing left it to not capture too broad exceptions and log warnings etc to verify what we are filtering is due to the filter vs other un related decoding/cv2 issues etc
| idx, image_bytes = item | ||
| if image_bytes is None: | ||
| return (idx, False) | ||
| image = _image_bytes_to_array(image_bytes) |
There was a problem hiding this comment.
Okay, please file an issue please so that we can track it . We can do this in the next release but we should track the issue.
|
/ok to test e8e3faa |
|
/ok to test abae9db |
|
/ok to test dc69766 |
Description
This pull request introduces new filtering capabilities for interleaved datasets.
The primary additions are the following four filters:
cv2.Laplacian) of an image, which acts as a measure of sharpness. If the variance falls below a specified threshold, the image is classified as blurry and filtered out.Usage
Checklist