Repository navigation
Declare embedding_dim and drop the default tag vocabulary for OpenCLIP RN101 - #43
Merged
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two changes to the OpenCLIP RN101 package, both about making the manifest describe the model instead of leaving darktable to assume things.
The first declares
embedding_dim: 512. darktable sizes its embedding buffers and the vector index column from the model's output width but had no way to learn it, so it hardcoded 512 – which is how a model of a different width could be truncated into an index built for another embedding space. darktable now reads this attribute and refuses a model that does not declare it rather than falling back to a default. That is a coordinated change: the darktable side is a separate PR, and packages built before this key need rebuilding or the updated darktable will refuse them.The second removes the 86-tag default vocabulary. It was the cold-start path for auto-tagging, but darktable replaced it with centroids built from a user's own tagged images, which describe what a photographer means by a tag far better than a generic label does. The model centroids are no longer applied, and importing them only added 86 unused names to the tag dictionary. Dropping the vocabulary takes the text encoder out of conversion entirely, so
convert.pyexports the image encoder alone andtags.jsonis gone from the package. The demo has nothing to overlay without tags, so it writes the embedding vector as JSON instead, and the SDK gains a.jsonoutput extension for the task.samples/embedalso moves tosamples/embedding, which the earlier task rename missed –run_demoresolvessamples/<task>, so it had been finding no samples for this model and skipping every image without failing.I ran the rewritten demo against the already-built
model.onnxand it returns dim 512 at norm 1.0000, which confirms both the declared dimension and the baked-in L2 normalisation. Both scripts compile. I did not re-run the conversion, so the ONNX produced by the editedconvert.pyis unverified, and I did not run the SDK demo runner end to end. No dependency changes.Written with AI assistance.