Skip to content

Refactor model URL constants #382

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

Conversation

msluszniak
Copy link
Member

@msluszniak msluszniak commented Jun 9, 2025

Description

This PR refactors URLs for models. This also exposed great inconsistencies in naming across hugging face repository:

  • Only for llama3_2-3B_qat_lora.pte there is - between name and number of parameters, while for the rest is _.
  • For WHISPER_TINY_DECODER en is concatenated in the first part of URL with . and not with - as in the rest of examples.
  • There is word all in efficientnet_v2_s_coreml_all and only for that model.
  • For some of the models initial name is concatenated with - and the model name as well - phi-4-mini and phi-4-mini_bf16.pte while in others there is - and _ e.g. qwen-2.5 and qwen2_5_3b_8da4w.pte.
  • Some model names adds backend info at the beginning: xnnpack_whisper_encoder.pte and some on the end or almost end: style_transfer_udnie_xnnpack.pte.
  • ssdlite320-mobilenet-v3-large v3 here is concatenated with - but in model name not: ssdlite320-mobilenetv3-large.pte.
  • For craft detector number means size of the intput tensor while for the other suffixed numbers means model sizes.
  • Some models B and M meaning billion and million of parameters is capitalized in both first part of url and name: smolLm-2-135M and smolLm2_135M_bf16.pte and for some not: qwen-2.5-1.5B and qwen2_5_0_5b_8da4w.pte.

And there are even more, but you will spot them while correcting rest. I think that there is need to make this unified. And then I will be able to refactor this even more.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update (improves or adds clarity to existing documentation)

Tested on

  • iOS
  • Android

Testing instructions

No apply

Screenshots

No apply

Related issues

No apply

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

@msluszniak msluszniak self-assigned this Jun 9, 2025
Copy link
Contributor

@chmjkb chmjkb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

review the conflict and we can merge!

@msluszniak
Copy link
Member Author

I'm thinking about adding model name before each block e.g.:

const LLAMA3_2_MODEL_NAME = 'llama-3.2';
export const LLAMA3_2_3B = `${BASE_URL_PREFIX}-${LLAMA3_2_MODEL_NAME}/${VERSION_TAG}/llama-3.2-3B/original/llama3_2_3B_bf16.pte`;
export const LLAMA3_2_3B_QLORA = `${BASE_URL_PREFIX}-${LLAMA3_2_MODEL_NAME}/${VERSION_TAG}/llama-3.2-3B/QLoRA/llama3_2-3B_qat_lora.pte`;

and moving the problem with naming inconsistencies into separate issue. What do you think @chmjkb?

@chmjkb
Copy link
Contributor

chmjkb commented Jun 25, 2025

I'm thinking about adding model name before each block e.g.:

const LLAMA3_2_MODEL_NAME = 'llama-3.2';
export const LLAMA3_2_3B = `${BASE_URL_PREFIX}-${LLAMA3_2_MODEL_NAME}/${VERSION_TAG}/llama-3.2-3B/original/llama3_2_3B_bf16.pte`;
export const LLAMA3_2_3B_QLORA = `${BASE_URL_PREFIX}-${LLAMA3_2_MODEL_NAME}/${VERSION_TAG}/llama-3.2-3B/QLoRA/llama3_2-3B_qat_lora.pte`;

and moving the problem with naming inconsistencies into separate issue. What do you think @chmjkb?

I think its fine if you change that too, and keep the naming stuff in a separate one.

@msluszniak
Copy link
Member Author

@chmjkb I'm closing this in favour of #430 . That PR is ready for review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants