Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
93 changes: 93 additions & 0 deletions internal/push/image_resolution.go
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,93 @@
package push

import (
"fmt"
"image"
"os"
"path/filepath"
)

// scanImageResolutions decodes each file's header (cheap, no full decode) and
// sorts it into three buckets — the shared core of the two ImageResolution
// previews: ValidateImages (the images/ dir) and ValidateMaskResolution (the
// masks/ dir). Keeping ONE decode-and-compare here is what lets the CLI mirror
// the ingestor's TWO ImageResolutionValidator instances for semantic
// segmentation (modalities/validators.py) without the rule drifting between
// them.
//
// - broken: zero-byte, unreadable, or undecodable files;
// - tooSmall: below the minW×minH floor (EITHER side under), when a floor is
// set — mirrors _meets_min_size;
// - mismatched: resolution != expectedW×expectedH (exact, no resize), when an
// expected size is set.
//
// expectedW/H or minW/H of 0 disables that comparison, exactly as documented on
// ValidateImages (see it for the parity rationale). Each offender string carries
// the file name and its dimensions for the caller's message.
func scanImageResolutions(paths []string, expectedW, expectedH, minW, minH int) (broken, tooSmall, mismatched []string) {
for _, path := range paths {
name := filepath.Base(path)
f, err := os.Open(path)
if err != nil {
broken = append(broken, fmt.Sprintf("%s (unreadable: %v)", name, err))
continue
}
cfg, _, err := image.DecodeConfig(f)
_ = f.Close()
if err != nil {
if st, serr := os.Stat(path); serr == nil && st.Size() == 0 {
broken = append(broken, name+" (empty file, 0 bytes)")
} else {
broken = append(broken, name+" (not a valid image — corrupt or unsupported format)")
}
continue
}
if minW > 0 && minH > 0 && (cfg.Width < minW || cfg.Height < minH) {
tooSmall = append(tooSmall, fmt.Sprintf("%s (%dx%d)", name, cfg.Width, cfg.Height))
}
if expectedW > 0 && expectedH > 0 && (cfg.Width != expectedW || cfg.Height != expectedH) {
mismatched = append(mismatched, fmt.Sprintf("%s (%dx%d)", name, cfg.Width, cfg.Height))
}
}
return broken, tooSmall, mismatched
}

// ValidateMaskResolution previews the ingestor's SECOND ImageResolutionValidator
// for semantic_segmentation — the one named "Mask Resolution Validator" with
// subdir="masks" (modalities/validators.py semantic_segmentation): it reads
// every PNG mask and rejects zero-byte / undecodable files, masks below the
// minimum-size floor, and any mask whose resolution differs from the expected
// target size. Masks are pixel-wise label maps, so they must share the images'
// resolution — the ingestor constructs this validator with the SAME
// expected_resolution (target_size) and min_size as the images'
// ImageResolutionValidator, and this preview is called with those same values.
//
// Before cli#352 the CLI validated only images/ resolution (ValidateImages) and
// never the masks, so a corrupt or mis-sized mask passed local preflight and
// then failed in-cluster after the full upload — the accept-then-reject the
// parity contract exists to prevent. The too-small floor takes precedence over
// the resolution mismatch, matching ValidateImages and the ingestor's ordering.
func ValidateMaskResolution(masks []string, expectedW, expectedH, minW, minH int) error {
const maxListed = 5
broken, tooSmall, mismatched := scanImageResolutions(masks, expectedW, expectedH, minW, minH)
if len(tooSmall) > 0 {
return fmt.Errorf(
"%d mask(s) are smaller than the %dx%d minimum you set with --min-size: %s. "+
"Provide larger masks, or lower the floor with --min-size, then re-run.",
len(tooSmall), minW, minH, TruncateList(tooSmall, maxListed))
}
if len(broken) > 0 {
return fmt.Errorf(
"%d mask(s) in masks/ can't be ingested: %s. The cluster reads every mask as a PNG "+
"and rejects these after the upload — fix or remove them and re-run.",
len(broken), TruncateList(broken, maxListed))
}
if len(mismatched) > 0 {
return fmt.Errorf(
"%d mask(s) don't match the %dx%d resolution the images use: %s. Semantic-segmentation "+
"masks are pixel-wise label maps, so each mask must be exactly the image size — the "+
"cluster validates this after the upload. Resize the masks to match and re-run.",
len(mismatched), expectedW, expectedH, TruncateList(mismatched, maxListed))
}
return nil
}
68 changes: 68 additions & 0 deletions internal/push/image_resolution_test.go
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
package push

import (
"os"
"path/filepath"
"strings"
"testing"
)

// TestValidateMaskResolution mirrors TestValidateImages for the semseg "Mask
// Resolution Validator" preview (cli#352): masks at the target size pass; a
// mask whose resolution differs is rejected (naming it and both sizes, which
// proves the dimensions were decoded — not merely that the file is unreadable);
// a zero-byte / corrupt mask is rejected; the min-size floor applies; and an
// empty set or a 0-expected size is a no-op.
func TestValidateMaskResolution(t *testing.T) {
dir := t.TempDir()
write := func(name string, body []byte) string {
p := filepath.Join(dir, name)
if err := os.WriteFile(p, body, 0o644); err != nil {
t.Fatal(err)
}
return p
}
good := write("a_mask.png", pngBytes(t, 32, 32))
wrong := write("b_mask.png", pngBytes(t, 48, 48))
zero := write("z_mask.png", nil)

if err := ValidateMaskResolution(nil, 32, 32, 0, 0); err != nil {
t.Errorf("empty mask set must pass: %v", err)
}
if err := ValidateMaskResolution([]string{good}, 32, 32, 0, 0); err != nil {
t.Errorf("32x32 mask against a 32x32 target rejected: %v", err)
}

// Wrong resolution: rejected, naming the file and BOTH sizes — the decode
// happened, so this is a genuine size mismatch, not a broken-file fallback.
err := ValidateMaskResolution([]string{good, wrong}, 32, 32, 0, 0)
if err == nil {
t.Fatal("48x48 mask against a 32x32 target must be rejected (cli#352)")
}
for _, want := range []string{"b_mask.png", "48x48", "32x32", "resolution"} {
if !strings.Contains(err.Error(), want) {
t.Errorf("mismatch error missing %q: %v", want, err)
}
}

// Zero-byte mask can't be ingested.
if err := ValidateMaskResolution([]string{good, zero}, 32, 32, 0, 0); err == nil {
t.Fatal("zero-byte mask must be rejected")
} else if !strings.Contains(err.Error(), "0 bytes") {
t.Errorf("zero-byte diagnosis missing: %v", err)
}

// 0-expected size skips the resolution comparison (auto-detect path).
if err := ValidateMaskResolution([]string{good, wrong}, 0, 0, 0, 0); err != nil {
t.Errorf("no expected size → no resolution rejection: %v", err)
}

// Min-size floor applies to masks too (same as images): a below-floor mask
// is rejected even with no target size.
tiny := write("tiny_mask.png", pngBytes(t, 16, 16))
if err := ValidateMaskResolution([]string{tiny}, 0, 0, 32, 32); err == nil {
t.Fatal("below-floor mask must be rejected")
} else if !strings.Contains(err.Error(), "minimum") {
t.Errorf("floor diagnosis missing: %v", err)
}
}
51 changes: 23 additions & 28 deletions internal/push/preflight.go
Original file line numberDiff line numberDiff line change
Expand Up@@ -18,7 +18,6 @@ import (
"encoding/csv"
"errors"
"fmt"
"image"
"io"
"math"
"os"
Expand DownExpand Up@@ -243,33 +242,10 @@ func CheckHasDataRows(path string) error {
// target_size uniformity error.
func ValidateImages(images []string, expectedW, expectedH, minW, minH int) error {
const maxListed = 5
var broken, tooSmall, mismatched []string
for _, path := range images {
name := filepath.Base(path)
f, err := os.Open(path)
if err != nil {
broken = append(broken, fmt.Sprintf("%s (unreadable: %v)", name, err))
continue
}
cfg, _, err := image.DecodeConfig(f)
_ = f.Close()
if err != nil {
if st, serr := os.Stat(path); serr == nil && st.Size() == 0 {
broken = append(broken, name+" (empty file, 0 bytes)")
} else {
broken = append(broken, name+" (not a valid image — corrupt or unsupported format)")
}
continue
}
if minW > 0 && minH > 0 && (cfg.Width < minW || cfg.Height < minH) {
tooSmall = append(tooSmall,
fmt.Sprintf("%s (%dx%d)", name, cfg.Width, cfg.Height))
}
if expectedW > 0 && expectedH > 0 && (cfg.Width != expectedW || cfg.Height != expectedH) {
mismatched = append(mismatched,
fmt.Sprintf("%s (%dx%d)", name, cfg.Width, cfg.Height))
}
}
// scanImageResolutions (image_resolution.go) is the shared decode-and-compare
// core — the SAME one ValidateMaskResolution runs over masks/, so the two
// ImageResolutionValidator previews can't drift (cli#352).
broken, tooSmall, mismatched := scanImageResolutions(images, expectedW, expectedH, minW, minH)
// Floor first: an image below the minimum size simply can't be trained
// on, so it's the most fundamental, actionable failure — data-ingestors
// #348 returns it ahead of the uniformity / target_size mismatch.
Expand DownExpand Up@@ -1444,6 +1420,16 @@ func PreflightDataset(spec SpecArgs, layout *LocalLayout) (notes []string, probl
if err := CheckSchemaColumns(header, spec.Schema, "the data CSV"); err != nil {
return nil, dataProblem(err)
}
// Per-value TYPE check (DataValidator preview, cli#352): a value that
// doesn't match its column's declared NUMERIC type — a non-numeric or
// fractional value in an INT column, a non-numeric value in a FLOAT
// column — is rejected by the ingestor's DataValidator after the table
// is created; CheckSchemaColumns only proves the columns EXIST. See
// CheckColumnValueTypes for the deliberately-narrow, never-over-reject
// scope (numeric types only).
if err := CheckColumnValueTypes(layout.LabelsCSV, spec.Schema, spec.Category); err != nil {
return nil, dataProblem(err)
}
// A bogus --label-column otherwise fails in-cluster only at READ
// time — after the table was created — leaving an orphaned table.
if err := CheckLabelColumn(header, spec.LabelColumn, "the data CSV"); err != nil {
Expand DownExpand Up@@ -1594,6 +1580,15 @@ func PreflightDataset(spec SpecArgs, layout *LocalLayout) (notes []string, probl
if err := CheckMaskIDColumn(layout.LabelsCSV); err != nil {
return nil, dataProblem(err)
}
// Mask resolution — the ingestor's SECOND ImageResolutionValidator
// ("Mask Resolution Validator", subdir=masks). Masks are pixel-wise
// label maps and must share the images' target size + min-size floor;
// a corrupt or mis-sized mask otherwise passes preflight and fails
// in-cluster after the upload (cli#352). Same expected/floor as the
// images (expW/expH/minW/minH), mirroring the factory.
if err := ValidateMaskResolution(layout.Sidecars["masks"], expW, expH, minW, minH); err != nil {
return nil, dataProblem(err)
}
missing, orphanFiles, cerr := CrossCheckLabels(layout.LabelsCSV, layout.Images, spec.Extension)
if cerr != nil {
return nil, dataProblem(cerr)
Expand Down
54 changes: 54 additions & 0 deletions internal/push/testdata/parity/cases.json
Original file line numberDiff line numberDiff line change
Expand Up@@ -841,6 +841,60 @@
"cli_verdict": "accept",
"ingestor_verdict": "reject",
"note": "DOCUMENTED under-preview: a negative 'time' value \u2014 TimeToEventValidator rejects in-cluster, but the CLI has no TTE time-column mirror, so the failure surfaces post-upload. Candidate for a future TTE preview."
},
{
"extension": ".jpg",
"target_size": [
32,
32
],
"csv": "labels.csv",
"name": "semseg-mask-res-mismatch",
"category": "semantic_segmentation",
"label_column": "label",
"cli_verdict": "reject",
"ingestor_verdict": "reject",
"note": "b_mask.png is 48x48 while the images (and target_size) are 32x32: the ingestor's SECOND ImageResolutionValidator (name 'Mask Resolution Validator', subdir 'masks') rejects it, and the CLI now previews it via ValidateMaskResolution (cli#352). Pairing + mask_id are valid, so mask resolution is the sole rejecter. Before cli#352 the CLI validated only images/ and accepted this, then it failed in-cluster after the upload."
},
{
"csv": "data.csv",
"name": "tabular-int-nonnumeric",
"category": "tabular_classification",
"label_column": "label",
"schema": {
"age": "INT",
"label": "VARCHAR(255)"
},
"cli_verdict": "reject",
"ingestor_verdict": "reject",
"note": "age is declared INT but data row 1 is 'abc': the ingestor's DataValidator rejects the non-numeric value (labels cat/dog keep diversity happy, so DataValidator is the sole rejecter), and the CLI now previews it via CheckColumnValueTypes (cli#352). Explicit schema so BOTH sides validate the same typed column."
},
{
"csv": "data.csv",
"name": "tabular-int-noninteger",
"category": "tabular_classification",
"label_column": "label",
"schema": {
"age": "INT",
"label": "VARCHAR(255)"
},
"cli_verdict": "reject",
"ingestor_verdict": "reject",
"note": "age is declared INT but data row 2 is '40.5': DataValidator's fractional-value check rejects it; CheckColumnValueTypes previews the same (cli#352). Pins the non-integer sub-check as distinct from the non-numeric one above."
},
{
"csv": "data.csv",
"name": "tabular-typed-ok",
"category": "tabular_classification",
"label_column": "label",
"schema": {
"age": "INT",
"price": "FLOAT",
"label": "VARCHAR(255)"
},
"cli_verdict": "accept",
"ingestor_verdict": "accept",
"note": "OVER-REJECT guard for cli#352: a fully-typed schema whose values match (INT age, FLOAT price incl. '1e3' scientific notation, VARCHAR label with 2 classes) must pass on BOTH sides. Proves CheckColumnValueTypes accepts numeric-looking values (the #188 direction) rather than over-rejecting a valid typed dataset."
}
]
}
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
filename,mask_id
a.jpg,a_mask.png
b.jpg,b_mask.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
age,label
30,cat
40.5,dog
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
age,label
abc,cat
40,dog
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
age,price,label
30,1e3,cat
40,2.5,dog
24 changes: 24 additions & 0 deletions internal/push/testdata/parity/goldens.json
Original file line numberDiff line numberDiff line change
Expand Up@@ -216,6 +216,14 @@
],
"verdict": "reject"
},
"semseg-mask-res-mismatch": {
"errors": [
"ImageResolutionValidator: Multiple image resolutions found: [(32, 32), (48, 48)]. All images must have the same resolution.",
"ImageResolutionValidator: Expected resolution: [32, 32]",
"ImageResolutionValidator: Resolution errors: ['masks/b_mask.png: (48, 48) (expected: [32, 32])']"
],
"verdict": "reject"
},
"semseg-missing-mask": {
"errors": [
"FilePairingValidator: 1 image(s) have no matching mask: ['b']"
Expand DownExpand Up@@ -295,6 +303,18 @@
],
"verdict": "reject"
},
"tabular-int-noninteger": {
"errors": [
"DataValidator: Column 'age' contains 1 non-integer values"
],
"verdict": "reject"
},
"tabular-int-nonnumeric": {
"errors": [
"DataValidator: Column 'age' contains 1 non-numeric value(s) at row 0."
],
"verdict": "reject"
},
"tabular-label-missing": {
"errors": [],
"verdict": "accept"
Expand DownExpand Up@@ -325,6 +345,10 @@
},
"verdict": "accept"
},
"tabular-typed-ok": {
"errors": [],
"verdict": "accept"
},
"tabular-varchar-numeric-labels": {
"errors": [],
"values": {
Expand Down
Loading
Loading