[RNE Rewrite] refactor!: add better model schema contract and validation logic - #1327
Conversation
ef56a3f to
cacfa78
Compare
|
@barhanc I updated the export script so know it should work correctly (at least I can export them my myself from Norbert's branch). The problem was in the newest version of transformers. Please give it a try. |
…ns (#1339) ## Description This PR improves the quality of native C++ error messages thrown to JavaScript, making it easier to diagnose issues without needing to read source code. This is a follow-up to error messages improvements introduced in #1327. ### Introduces a breaking change? - [ ] Yes - [x] No ### Type of change - [ ] Bug fix (change which fixes an issue) - [ ] New feature (change which adds functionality) - [ ] Documentation update (improves or adds clarity to existing documentation) - [x] Other (chores, tests, code style improvements etc.) ### Tested on - [ ] iOS - [ ] Android ### Testing instructions N/A ### Screenshots <!-- Add screenshots here, if applicable --> ### Related issues <!-- Link related issues here using #issue-number --> ### Checklist - [ ] I have performed a self-review of my code - [ ] I have commented my code, particularly in hard-to-understand areas - [ ] I have updated the documentation accordingly - [ ] My changes generate no new warnings ### Additional notes <!-- Include any additional information, assumptions, or context that reviewers might need to understand this PR. -->
42cf543 to
3f0a772
Compare
|
The models should be fixed now. I've updated the |
msluszniak
left a comment
There was a problem hiding this comment.
Now everything worked correctly, @benITo47 are you fine with these changes?
|
From what I've been reading, changes seem sane. One real gap for OCR: enumerated shapes are a per-dimension cross product. tensor_helpers.cpp fromJs validates each dim against its own EnumDim.choices independently, and dims.enum only exposes per-dim arrays. There's no whole-shape domain, and runtimeConstraints (eq/linear) are dim-relations, so they can't carve out a sparse set either. For example, our CoreML detectors ship sets like {640², 960², 1280², 1280×640} for PaddleOCR. To describe that in the new schema I can only give H:{640,960,1280}, W:{640,960,1280} - whose cross product also admits 640×1280, which the PTE never compiled. fromJs green-lights it, then CoreML rejects it at execute. While resizing to match input, im picking which shape to use (snap up or snap down), in that process I can actively select that illegal shape. To make the per-dim schema match the compiled model I'd have to either compile the full cross product (more ANE specializations, more coreML overhead, worse performance) or drop to square-only (lose the portrait bucket). The workaround is to hardcode the whole-shape list in the task and snap to it - but then the model doesn't self-describe its contract, code breaks when hardcoded list drifts from the export, and inspectModel shows a wrong over-permissive shape set. So at minimum this should be expressible in schema.ts - a whole-shape / tuple-enum domain (or constraint kind), whatever fits the model best. |
…ask specifications
…pers - Re-export schema utilities under a namespaced 'schema' export in index.ts - Re-export types flatly using 'export type *' from ./core/schema - Group constraint helpers under 'constr' object and inline DimRef object literals in task pipelines - Remove redundant static dimension constraint from Whisper task pipeline
…helper and inline DimRef objects
- Expand schema.h file-level docblock to detail the two-phase validation model (load-time validateSpec + runtime validateRuntimeConstraints), explain the two spec source paths (companion method vs MethodMeta fallback), and document output-dimension skipping behaviour pre-execution - Improve individual function docstrings for parseModelSpecJson, validateSpec, and validateRuntimeConstraints to reflect actual behaviour more precisely - Fix whisperSpeechToText decode method spec: replace named symbol strings for static constant dimensions with literal integers (1) to match schema semantics
…rce merge behavior
…rs and schema.cpp
… get_model_schema
4146707 to
bfcbf24
Compare
|
@msluszniak I've rebased, updated the models on SWM HF and tested them again (the ones I could on a simulator). If there is a green light I will merge this PR. |
|
There is a green light indeed, today I tested both coreml and mlx variants of uploaded files. |
#1327 replaced `validateModelSchema` with `validateSpec` and dropped the `get_dynamic_dims_forward` companion in favour of `get_model_schema`. - Declare `forward` as two spec variants: `dynamic`, whose sequence dim binds to the exported range, and `static`, whose sequence dim binds to the single exported constant. The window size falls out of whichever variant matched, replacing both the `inputTensorMeta` shape read and the `getMethodNames()` probe for the old companion method. - Declare the sequence length shared by both inputs and the logits as an equality runtime constraint, so a length mismatch is rejected before `execute` instead of surfacing as an internal backend error. - Snap the length buckets onto the exported range's grid (and start them at its lower bound) rather than assuming a step of 1 from zero.



Description
Intoduces a new powerful and extensible mechanism for validating model contracts both during the runtime execution (mitigating e.g. internal error crashes when two input tensors should have the same dimension during runtime) and pipeline construction (matching model contract against a specified interface).
get_model_schemareturning string-encoded JSON. No need for constructing auxiliary tensors for specifying dynamic dimensions, multiple companions for different kinds, etc.Introduces a breaking change?
Type of change
Tested on
Testing instructions
Models that use the companion method to get the JSON schema have not yet been updated on the SWM HF as they wait for the review of this PR. For testing you can use the models at https://huggingface.co/bhanc/scratch/tree/main/ptes.
Screenshots
Related issues
Closes #1323
Checklist
Additional notes
@bh/schema-workflowbranch.@nk/universal-exporterbranch. I would be grateful if you could take a look.