Skip to content

ML Model Download Progress & Logo#434

Merged
RemiLehe merged 1 commit into
BLAST-AI-ML:mainfrom
ax3l:topic-logo-model-download
May 7, 2026
Merged

ML Model Download Progress & Logo#434
RemiLehe merged 1 commit into
BLAST-AI-ML:mainfrom
ax3l:topic-logo-model-download

Conversation

@ax3l
Copy link
Copy Markdown
Member

@ax3l ax3l commented May 5, 2026

Simpler version of #433, subset ported over:

  • Show download progress from MLFlow
  • Add the Genesis AmSC Logo and make it link to MLFlow model catalogue

To do:

@ax3l ax3l force-pushed the topic-logo-model-download branch 2 times, most recently from 024d338 to 3e52bc6 Compare May 5, 2026 05:48
Comment thread dashboard/model_manager.py Outdated
@ax3l ax3l force-pushed the topic-logo-model-download branch 2 times, most recently from 385dda1 to 2f37129 Compare May 6, 2026 20:29
Comment thread dashboard/model_manager.py Outdated
Simpler version of BLAST-AI-ML#433:

- Show download progress from MLFlow
- Add the Genesis AMSC Logo and make it link to MLFlow model catalogue
@ax3l ax3l force-pushed the topic-logo-model-download branch from 2f37129 to 33f2286 Compare May 6, 2026 20:32
@ax3l ax3l requested review from EZoni and RemiLehe May 6, 2026 21:01
@ax3l ax3l added the dashboard Changes related to the dashboard label May 6, 2026
@EZoni
Copy link
Copy Markdown
Member

EZoni commented May 6, 2026

Posting here some comments taken out of a Codex review:

  1. dashboard/model_manager.py: the AmSC logo image is duplicated once inside a link and once outside it. A tiny helper or single image with conditional link behavior would make this section smaller.

  2. dashboard/model_manager.py: AMSC_MLFLOW_MODEL_URL_EXPR duplicates model_type_dict as a Vue ternary. Build the model URL server-side with build_mlflow_model_name() and store it in state, then bind href directly. That removes drift risk and cuts down the expression.

Main advantage: The model URL now has one source of truth.

  1. dashboard/app.py + dashboard/model_manager.py: is_model_available_on_mlflow() adds a remote MLflow query, but the "not available" path still calls update(), which constructs ModelManager and attempts the MLflow load anyway. This looks redundant: remove the preflight helper and either always show the download indicator for configs with mlflow.tracking_uri, or keep all MLflow availability/load handling inside ModelManager.

Main advantage: MLflow loading now has one source of truth, ModelManager and the existing load path.

  1. dashboard/model_manager.py + dashboard/app.py: the MLflow progress implementation monkey-patches global MLflow classes, while multiple download tasks can be started without cancelling/serializing the previous one. Overlapping downloads can restore each other's patched classes in the wrong order or leave stale progress behavior installed. The more minimal and safer option is probably an indeterminate model_downloading bar only; it would delete the private MLflow imports and most of this context manager.

Main advantage: the dashboard no longer modifies MLflow’s global internals just to show a % progress.

They seem legit improvements to me and could be either separate commits in this PR or separate follow-up PRs. I already tested local patches for each of them, which do make the code a bit more minimal, but will wait to know what others think before going further. We can also discuss them in person if needed - whatever works best.

@ax3l
Copy link
Copy Markdown
Member Author

ax3l commented May 7, 2026

I agree with 1-3, feel free to push commits on this PR @EZoni

4: I rather would cancel in-flight downloads. If I wanted an unhelpful, indetermine progress bar I could have avoided a lot of the lifting here. So either ignore or add a cancel.

@ax3l
Copy link
Copy Markdown
Member Author

ax3l commented May 7, 2026

For 3 the pre-flight helper is not clear to me. I definitely want to have a is_model_available_on_mlflow() function, otherwise the async code and exception handling gets very messy and error prone.

You want to minimize all exception handling in async functions, because they are notoriously brittle and hard to debug in python.

So do 3. Carefully, it's overly eager in its recommendation.

@RemiLehe RemiLehe merged commit 2d9b415 into BLAST-AI-ML:main May 7, 2026
3 checks passed
@ax3l ax3l mentioned this pull request May 7, 2026
@ax3l ax3l deleted the topic-logo-model-download branch May 7, 2026 17:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dashboard Changes related to the dashboard

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants