Tolerate broken image files per-item during embedding#1564
Conversation
LIG-10034. A single un-decodable image no longer crashes the whole embedding batch. Broken files are skipped per-item, recorded as broken in a per-run FileOutcomeReport, and left without an embedding row (no placeholder); the rest of the batch still embeds. The all-failed rule (LIG-10029) still raises when every attempted file fails. - Add shared open_image_as_rgb decode boundary that translates decode failures into BrokenInputFileError (raise ... from e). Both the full-image and image-crop paths route decodes through it, one typed-error contract. - image_embedding: decode runs inside the DataLoader worker, so __getitem__ returns the typed error alongside the input index; the main-process collation re-raises it inside report.track so the outcome is recorded there. Good items in the batch are still stacked and encoded. - image_crop_embedding: decode each source file inside report.track; a broken source file is recorded once and all its crops are skipped. - embedding_manager: own the FileOutcomeReport, apply raise_if_all_failed + log_summary (embed_annotations shares one report across chunks). compute_image_embedding raises on a broken single file. Because results are keyed by sample id (the prefactor), skipping a broken file just omits its key from the result — no index bookkeeping. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 187035ee9c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| except (OSError, UnidentifiedImageError) as exc: | ||
| raise BrokenInputFileError(f"Could not decode image '{filepath}'.") from exc |
There was a problem hiding this comment.
OSError also includes FileNotFoundError, so when an embedding job references a deleted image (or an fsspec backend raises not-found while opening), this helper converts it to BrokenInputFileError; report.track then suppresses it and continues without an embedding row. That widens the new tolerance from undecodable images to missing/IO failures and misreports them as broken, despite the existing outcome contract having MissingInputFileError for missing paths. Check existence before this catch or narrow the caught exceptions.
Useful? React with 👍 / 👎.
| from lightly_studio.core.file_outcome_report import FileOutcomeReport | ||
| from lightly_studio.dataset.embedding_generator import BatchedEmbeddingResult, ImageCrop | ||
| from lightly_studio.dataset.image_embedding import EmbeddingContext | ||
| from lightly_studio.dataset.image_embedding import EmbeddingContext, open_image_as_rgb |
There was a problem hiding this comment.
The Python guideline in AGENTS.md says functions should be imported via the containing module and called with dot notation, while classes may be imported directly. This new direct function import violates that convention; import the image_embedding module and call image_embedding.open_image_as_rgb(...) instead.
Useful? React with 👍 / 👎.
1 participant
What
LIG-10034. A single un-decodable image no longer crashes the whole embedding batch. Broken files are skipped per-item, recorded as broken in a per-run
FileOutcomeReport, and left without an embedding row (no placeholder); the rest of the batch still embeds. The all-failed rule (LIG-10029) still raises when every attempted file fails.How
open_image_as_rgbtranslates decode failures intoBrokenInputFileError(raise ... from e). Both the full-image and image-crop paths route decodes through it — one typed-error contract, not three bespoke catch-alls. Unexpected exceptions propagate.__getitem__returns the typed error alongside the input index; the main-process collation re-raises it insidereport.trackso the outcome is recorded there. Good items in the batch are still stacked and encoded.report.track; a broken source file is recorded once and all its crops are skipped.FileOutcomeReport, appliesraise_if_all_failed+log_summary(embed_annotationsshares one report across chunks).compute_image_embeddingraises on a broken single file.Because results are keyed by sample id (the prefactor), skipping a broken file just omits its id from
keys— no index bookkeeping.Acceptance criteria
BrokenInputFileErrorrouted through the helper.🤖 Generated with Claude Code