Add normalization with ai/ prefix for official models by ericcurtin · Pull Request #126 · docker/model-distribution

ericcurtin · 2025-09-02T19:33:32Z

So we can set our own defaults

xenoscopic · 2025-09-02T20:19:26Z

I think mdltool is just for development work and testing, but I think the general sentiment of the code is correct and I think this is the right repo. I think you'd want to do it here (and possibly here and here). @ilopezluna or @ekcasey would know best.

ericcurtin · 2025-09-03T10:34:57Z

@xenoscopic WDYT? I repushed

ericcurtin · 2025-09-03T11:38:09Z

@xenoscopic I do have a question, I want to test this end to end manually, model-cli + model-distribution, my attempt is this:

cd model-cli
go mod edit -replace github.com/docker/model-distribution=../model-distribution
go mod edit -replace github.com/docker/model-runner=../model-runner
go mod vendor
go mod tidy
make build

doesn't seem to work. I tried a couple of other things, didn't get anything to work. Searching for an efficient end to end workflow.

Also another question along these lines, how can I see the prints/logs from each component in an end to end test like this if I do get it working.

Preferably I'd like to run without containers. If possible. Rather than rebuild and re-run a whole container, just rebuild the binaries required during development.

I tried to run ./model-runner also outside the container, but ./model-cli seems insistent on using the version in containers.

I'm on Linux at the moment.

I also get this when I try and build container images:

ERROR: failed to build: Multi-platform build is not supported for the docker driver.
Switch to a different driver, or turn on the containerd image store, and try again.
Learn more at https://docs.docker.com/go/build-multi-platform/
make: *** [Makefile:39: docker-build] Error 1

ekcasey · 2025-09-04T16:42:07Z

Hey @ericcurtin

I tried to run ./model-runner also outside the container, but ./model-cli seems insistent on using the version in containers.

We should document this but model-cli will respect the MODEL_RUNNER_HOST evironment variable. So if you want to run a dev version of model runner and use the CLI to interact with it you can do something like:

in model-runner repo:

MODEL_RUNNER_PORT=3000 make run # in model-runner repo

And then set MODEL_RUNNER_HOST=http://localhost:3000 in the terminal where you are testing the CLI.

WRT the substance of this PR I am onboard with the idea of ai as the default namespace. It's worth noting the library is the default namespace right now (this happens implicitly through the parsing library we are using. So this would be a breaking change if there were any models pushed to library, but AFAIK there are so it should be fine 🙂

However, I think this implementation would result in the default namespace always appearing in the tag in the list. E.g. if you docker model pull smollm2 you will see ai/smollm2 in the model list instead of smollm2. This isn't ideal b/c it's not directly analogous to docker pull (where library is inferred but not explicitly added to the reference) or to our handling of the default registry (docker.io). I also worry that given we use the reference parsing library in many places this implementation could lead to errors if we are explicitly adding the default in some places and not in other.

I think what we we want to do instead is:

pull from the default namespace if no registry or namespace is specificed but store the tag as given (without prepending default repo or default registry)
when looking up an artifact from the store by tag ensure <model>, ai/<model> docker.io/ai/<model> can all be used to look up the model regardless of which tag was given when it was pulled.

I think the easiest way to get the behavior we want without causing inconsistencies would be set the default namespace for the parsing library to use or supply a WithDefaultNamespace option when parsing references in pull and comparison code, unfortunately no such option exists and the defaultNamespace is not exported. Maybe we could either contribute more config options upstream or wrap/replace the parsing code so we have a utility that applies the correct default namespace that we can use in places of our current usage of name.ParseReference and name.ParseTag?

So we can set our own defaults Signed-off-by: Eric Curtin <ericcurtin17@gmail.com>

ericcurtin · 2025-09-04T21:06:55Z

@ekcasey @xenoscopic how does this look?

xenoscopic

Looks pretty good to me, happy to +1 once it passes CI. We really should get a +1 from @ilopezluna or @ekcasey though since they know the code much better.

xenoscopic · 2025-09-16T18:06:43Z

registry/normalize.go

+	normalizedRef := normalizeReference(reference)
+
+	// Validate the normalized reference
+	if _, err := name.ParseReference(normalizedRef); err != nil {


It would be nice if we could avoid doing this twice, maybe we could just have a NoramlizeAndParseReference() function since most cases call ParseReference directly after Normalize.

xenoscopic · 2025-09-16T18:13:07Z

registry/normalize.go

+
+	// Simple name - add default domain and official prefix
+	return defaultDomain + "/" + officialRepoPrefix + name
+}


The files are missing a trailing newline, which may be what's failing the linter. You'll need to do a go fmt ./registry/... on the PR.

xenoscopic · 2025-09-16T18:17:35Z

registry/integration_test.go

+			if err == nil {
+				t.Error("expected error when trying to connect to non-existent registry")
+			}


It would be a bit nicer if you could use errors.Is here to look for some sort of sentinel error (or, failing that, match on err.Error(), even though it's a bit fragile), otherwise we don't really know that the failure came after normalization.

xenoscopic · 2025-09-16T18:25:04Z

registry/normalize_test.go

+	"testing"
+)
+
+func TestNormalize(t *testing.T) {


Can you also add some tests for local registries, like localhost:5000/...?

ericcurtin · 2025-09-16T18:43:43Z

Happy to keep this post monorepo merge also @xenoscopic don't want to block that

xenoscopic · 2025-09-16T18:51:10Z

@ericcurtin no worries, I think we can get it in before then. Most of my comments are just suggestions, I think the go fmt is the only "hard" blocker.

ilopezluna · 2025-09-17T09:10:45Z

I found minor differences which I think are related with @ekcasey comment. For example having ai/llama3.2 locally, when I execute:
MODEL_RUNNER_HOST=http://localhost:13434 docker model pull ai/llama3.2:latest
I can see how l.modelManager.IsModelInStore(model) returns true. This is because store.HasTag(tag string) returns true.
However by doing:
docker model pull llama3.2
l.modelManager.IsModelInStore(model) returns false, so at least in the GetRequiredMemoryForModel the behavior is different.

I'm not sure of the implications of this difference but it seems safer if we also consider the comparison as @ekcasey mentioned:

I think the easiest way to get the behavior we want without causing inconsistencies would be set the default namespace for the parsing library to use or supply a WithDefaultNamespace option when parsing references in pull and comparison code,

I've run into more issues, but I think its because something must be updated in the model-cli first, sharing just in case:

$ MODEL_RUNNER_HOST=http://localhost:13434 docker model ls
MODEL NAME   PARAMETERS  QUANTIZATION    ARCHITECTURE  MODEL ID      CREATED       SIZE
ai/llama3.2  3.21 B      IQ2_XXS/Q4_K_M  llama         436bb282b419  5 months ago  1.87 GiB
llama3.2     3.21 B      IQ2_XXS/Q4_K_M  llama         436bb282b419  5 months ago  1.87 GiB

$ MODEL_RUNNER_HOST=http://localhost:13434 docker model run llama3.2
Failed to inspect model: invalid model name: llama3.2

Finally a comment about #126 (comment), sorry @xenoscopic for missing your message! mdltool is used to publish models: https://github.com/docker/model-publisher/blob/a211abcd9c4dc166cc9895e8b56ba1dea7598891/Dockerfile#L15

ilopezluna · 2025-09-23T11:31:34Z

We will address it in the mono repo, so closing this one

ericcurtin mentioned this pull request Sep 2, 2025

Make "ai/" the default namespace docker/model-cli#136

Open

ericcurtin force-pushed the ai-default-namespace branch from aec62fb to c448de9 Compare September 3, 2025 10:33

This was referenced Sep 4, 2025

Implement "ai/" as the default namespace for model references ericcurtin/model-distribution#2

Draft

Implement configurable default namespace for model references ericcurtin/model-distribution#3

Draft

ericcurtin force-pushed the ai-default-namespace branch from c448de9 to fba85f9 Compare September 4, 2025 21:04

ericcurtin changed the title ~~Make "ai/" the default namespace~~ Add normalization with ai/ prefix for official models Sep 4, 2025

Add normalization with ai/ prefix for official models

697724e

So we can set our own defaults Signed-off-by: Eric Curtin <ericcurtin17@gmail.com>

ericcurtin force-pushed the ai-default-namespace branch from fba85f9 to 697724e Compare September 4, 2025 21:06

xenoscopic reviewed Sep 16, 2025

View reviewed changes

ilopezluna closed this Sep 23, 2025

Comments

Conversation

ericcurtin commented Sep 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

xenoscopic commented Sep 2, 2025

Uh oh!

ericcurtin commented Sep 3, 2025

Uh oh!

ericcurtin commented Sep 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ekcasey commented Sep 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ericcurtin commented Sep 4, 2025

Uh oh!

xenoscopic left a comment

Choose a reason for hiding this comment

Uh oh!

xenoscopic Sep 16, 2025

Choose a reason for hiding this comment

Uh oh!

xenoscopic Sep 16, 2025

Choose a reason for hiding this comment

Uh oh!

xenoscopic Sep 16, 2025

Choose a reason for hiding this comment

Uh oh!

xenoscopic Sep 16, 2025

Choose a reason for hiding this comment

Uh oh!

ericcurtin commented Sep 16, 2025

Uh oh!

xenoscopic commented Sep 16, 2025

Uh oh!

ilopezluna commented Sep 17, 2025

Uh oh!

ilopezluna commented Sep 23, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

ericcurtin commented Sep 2, 2025 •

edited

Loading

ericcurtin commented Sep 3, 2025 •

edited

Loading

ekcasey commented Sep 4, 2025 •

edited

Loading