Skip to content

[BUG] version 3 todos #189

Description

@davidkoski

TODOs for the version 3 API:

Download jinja files

calls to downloader should also download jinja files, e.g. ModelAdapterFactory:

            adapterDirectory = try await downloader.download(
                id: id, revision: revision,
                matching: ["*.safetensors", "*.json"],

should be:

            ["*.safetensors", "*.json", "*.jinja"]

Note: some already do

package let tokenizerDownloadPatterns = ["*.json", "*.jinja"]
package let modelDownloadPatterns = ["*.safetensors"] + tokenizerDownloadPatterns

Macros need fully qualified names to avoid collisions

fully qualify the type names in the macros, e.g. Progress -> Foundation.Progress

The LoRATrainingExample has a local "Progress" type that collides inside the macro.

MLXEmbedders has colliding ModelConfiguration, etc.

In embedder-tool there are uses of:

  • ModelConfiguration
  • ModelContainer

I think we got away with this before because the code was only importing MLXEmbedders. Now, in order to provide Downloader and Tokenizer we have to import MLXLMCommon, which has a different definition.

In order to compile, the code would need fully qualified names for these types to disambiguate.

Ideas:

  • can we just use the MLXLMCommon form? I know we looked at that earlier and it wasn't trivial, but it might be the best way
  • use a different name in MLXEmbedders -- more breaking change, so not ideal
  • do we need to import MLXLMCommon -- maybe re-export types? this might be sweeping it under the rug

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions