feat: allow to import local models #771

lstocchi · 2024-04-02T14:53:34Z

What does this PR do?

It allows to import .gguf models that have already been downloaded by the user and that are on his machine.
After imported, the user can start a service, open its folder and delete it

Screenshot / video of UI

What issues does this PR fix or reference?

it resolves #173

Fixes #814

How to test this PR?

download a gguf model which is not listed in the catalog (e.g. take the first ones from here https://huggingface.co/TheBloke/Mistral-7B-Instruct-v0.1-GGUF/tree/main
go to models, import, select the gguf file
once imported, try to play with it (inference server + playground)
then delete it (it will be removed from catalog and disk)

slemeur · 2024-04-02T16:28:28Z

Nice!

lstocchi · 2024-04-03T10:14:18Z

There is an issue when you delete a model having an inference server using it and you stop/start ai lab again. Fixing it

packages/shared/src/uri/Uri.ts

axel7083

Nice feature ! I added some comment

packages/backend/src/managers/catalogManager.ts

packages/backend/src/managers/inference/inferenceManager.ts

axel7083 · 2024-04-03T12:44:26Z

packages/backend/src/managers/catalogManager.ts

+      });
+    }
+
+    const customCatalog = path.resolve(this.appUserDirectory, 'catalog.json');


I think we should have two catalog here. I am not in favor of overwriting the native catalog, I feel it would be more appropriate to have some kind of user-models.json in the appUserDirectory what do you think ?

From my point of view the operation that the user is doing here it is just updating the custom catalog.json file that we already support. It is just that we guide them. But if someone wants to add a new model manually we are already saying that they need to create a catalog.json file (copy/paste the assets/ai.json) inside the ai-lab folder and update it.

So i wouldn't create a new file as it may overcomplicate stuff, but let's see what others think

How do you handle the case where we update on our side the catalog ? Are we losing the user imported models ?

When importing, if the catalog.json file already exists, it just adds the new models in there and update the file on disk. If it does not exists it creates it on disk by copying the content from the native ai.json and adding the imported models.

So if the user wants to update the catalog after having imported models he would work as before. Open the catalog.json file which would also contain the imported models and edit it.

What if some day we want to remove a model ? like we did with llama2 ? the mechanism you describe would not be suitable ?

This is the same thing that may happen today. If i create my custom catalog.json and we update ai.json, ai-lab will still point to the custom catalog. We have to refactor how the custom catalog + ai.json work or inform the user any update. Opening an issue

Okey good to have follow up issue thanks !

What about having the 2 files but merging them instead of choosing one over the other

axel7083

Almost looking good just some stuff I find out while testing it

Breadcrumb

Could we have the breadcrumb allowing to go back to models page ?

Import timeout

Taking very long time to select a file in explorer gave a timeout

We probably need to find out some kind of way to get back this value and update the form. (This could be skipped, but would need to be address at some point)

Selected file

Maybe I should not be able to select files inside the models folder ?

If I import a model from the AI-Studio, it gets weird

lstocchi · 2024-04-03T23:49:46Z

Nice catches!!

Almost looking good just some stuff I find out while testing it

Breadcrumb

Done

Import timeout

Taking very long time to select a file in explorer gave a timeout

I'll give a look on desktop side as the timeout is really tight. For the moment i added an error message if something unexpected happen so the user is informed.

Selected file

Maybe I should not be able to select files inside the models folder ?

Added a check to verify that the selected model is not already in the catalog.

jeffmaury · 2024-04-04T08:31:36Z

The size information is missing and I'm wondering how the RAM usage is computed

jeffmaury

The timeout issue is to be fixed because it breaks the user experience

lstocchi · 2024-04-04T08:40:08Z

The size information is missing and I'm wondering how the RAM usage is computed

The RAM usage is = size of the file as it seems to me that that is the value we use in the catalog, no?

lstocchi · 2024-04-04T09:40:18Z

@jeffmaury timeout removed and size + creation date added

packages/shared/src/messages/MessageProxy.ts

feloy · 2024-04-04T10:20:22Z

The size information is missing and I'm wondering how the RAM usage is computed

The RAM usage is = size of the file as it seems to me that that is the value we use in the catalog, no?

No, the ram usage does not come from the file size. For the models in the catalog, it is computed with the script in .github/workflows/compute-model-sizes.yaml (see #674)

lstocchi · 2024-04-04T10:35:18Z

The size information is missing and I'm wondering how the RAM usage is computed

The RAM usage is = size of the file as it seems to me that that is the value we use in the catalog, no?

No, the ram usage does not come from the file size. For the models in the catalog, it is computed with the script in .github/workflows/compute-model-sizes.yaml (see #674)

Ah I completely missed that part. Thanks for telling me. Removed the memory property for the moment then. 👍

packages/shared/src/models/NoTimeoutChannels.ts

packages/frontend/src/pages/ImportModels.svelte

packages/frontend/src/lib/table/model/ModelColumnName.svelte

jeffmaury · 2024-04-04T12:48:32Z

The size information is missing and I'm wondering how the RAM usage is computed

The RAM usage is = size of the file as it seems to me that that is the value we use in the catalog, no?

No, the ram usage does not come from the file size. For the models in the catalog, it is computed with the script in .github/workflows/compute-model-sizes.yaml (see #674)

Ah I completely missed that part. Thanks for telling me. Removed the memory property for the moment then. 👍

I would rather default the memory field to the file size

axel7083 · 2024-04-04T12:49:24Z

The size information is missing and I'm wondering how the RAM usage is computed

The RAM usage is = size of the file as it seems to me that that is the value we use in the catalog, no?

No, the ram usage does not come from the file size. For the models in the catalog, it is computed with the script in .github/workflows/compute-model-sizes.yaml (see #674)

Ah I completely missed that part. Thanks for telling me. Removed the memory property for the moment then. 👍

I would rather default the memory field to the file size

I think this is misleading and does not reflect reality

vrothberg · 2024-04-04T12:58:53Z

If we don't have the information about the resource constraints of a model, I wouldn't guess it. Maybe we can display NA (not available) for imported models?

jeffmaury · 2024-04-04T14:21:56Z

The size information is missing and I'm wondering how the RAM usage is computed

The RAM usage is = size of the file as it seems to me that that is the value we use in the catalog, no?

No, the ram usage does not come from the file size. For the models in the catalog, it is computed with the script in .github/workflows/compute-model-sizes.yaml (see #674)

Ah I completely missed that part. Thanks for telling me. Removed the memory property for the moment then. 👍

I would rather default the memory field to the file size

I think this is misleading and does not reflect reality

Both are in the same order of magnitude (file size is a little bit bigger) and as we will check the machine size based on this data, I would rather ask a little bit of additional memory than risking failures

axel7083

LGTM ! 🚀 thanks you for your patience and the fixes, great job!

Signed-off-by: lstocchi <lstocchi@redhat.com>

… on the catalog Signed-off-by: lstocchi <lstocchi@redhat.com>

… it when modelManager is disposed Signed-off-by: lstocchi <lstocchi@redhat.com>

…something unexpected happen when adding a new model and fix css inputs Signed-off-by: lstocchi <lstocchi@redhat.com>

…nels Signed-off-by: lstocchi <lstocchi@redhat.com>

Signed-off-by: lstocchi <lstocchi@redhat.com>

Co-authored-by: axel7083 <42176370+axel7083@users.noreply.github.com> Signed-off-by: Luca Stocchi <49404737+lstocchi@users.noreply.github.com>

Signed-off-by: lstocchi <lstocchi@redhat.com>

lstocchi · 2024-04-09T17:25:13Z

@jeffmaury set the memory value equals to the file size as you said. Let me know if there is something else to change.
Also rebased.

Signed-off-by: lstocchi <lstocchi@redhat.com>

jeffmaury

LGTM

lstocchi force-pushed the i173 branch 2 times, most recently from 6e4bc2f to 869ee6e Compare April 3, 2024 09:16

lstocchi marked this pull request as ready for review April 3, 2024 09:17

lstocchi requested review from benoitf and a team as code owners April 3, 2024 09:17

lstocchi requested review from jeffmaury and vrothberg April 3, 2024 09:17

axel7083 reviewed Apr 3, 2024

View reviewed changes

packages/shared/src/uri/Uri.ts Show resolved Hide resolved

axel7083 reviewed Apr 3, 2024

View reviewed changes

lstocchi requested a review from axel7083 April 3, 2024 23:49

jeffmaury reviewed Apr 4, 2024

View reviewed changes

lstocchi force-pushed the i173 branch from bffa011 to 1eb24d9 Compare April 4, 2024 09:39

lstocchi requested a review from jeffmaury April 4, 2024 09:40

lstocchi mentioned this pull request Apr 4, 2024

Downloading model fails when using a user's catalog #814

Closed

axel7083 requested changes Apr 4, 2024

View reviewed changes

packages/shared/src/messages/MessageProxy.ts Outdated Show resolved Hide resolved

lstocchi requested a review from axel7083 April 4, 2024 10:35

axel7083 reviewed Apr 4, 2024

View reviewed changes

packages/shared/src/models/NoTimeoutChannels.ts Outdated Show resolved Hide resolved

lstocchi force-pushed the i173 branch from 894b352 to fd74cef Compare April 4, 2024 10:39

lstocchi requested a review from axel7083 April 4, 2024 10:40

axel7083 reviewed Apr 4, 2024

View reviewed changes

packages/frontend/src/pages/ImportModels.svelte Outdated Show resolved Hide resolved

axel7083 reviewed Apr 4, 2024

View reviewed changes

packages/frontend/src/pages/ImportModels.svelte Show resolved Hide resolved

packages/frontend/src/pages/ImportModels.svelte Outdated Show resolved Hide resolved

axel7083 reviewed Apr 4, 2024

View reviewed changes

packages/frontend/src/lib/table/model/ModelColumnName.svelte Outdated Show resolved Hide resolved

lstocchi requested a review from axel7083 April 4, 2024 14:18

axel7083 approved these changes Apr 4, 2024

View reviewed changes

lstocchi and others added 14 commits April 9, 2024 19:22

feat: allow to import local models

64209d6

Signed-off-by: lstocchi <lstocchi@redhat.com>

fix: add tests

5d9ffb3

Signed-off-by: lstocchi <lstocchi@redhat.com>

fix: skip inference server which are using a model which do not exist…

947c986

… on the catalog Signed-off-by: lstocchi <lstocchi@redhat.com>

fix: create disposable when adding catalogUpdate listener and dispose…

b297baf

… it when modelManager is disposed Signed-off-by: lstocchi <lstocchi@redhat.com>

fix: show inference servers if they have no models, display error if …

8f93e2c

…something unexpected happen when adding a new model and fix css inputs Signed-off-by: lstocchi <lstocchi@redhat.com>

fix: add size and creation date and skip settimeout for specific chan…

76f5664

…nels Signed-off-by: lstocchi <lstocchi@redhat.com>

fix: add tests

ab514c8

Signed-off-by: lstocchi <lstocchi@redhat.com>

fix: move notimeoutchannels to external file and remove memory prop

a863fb1

Signed-off-by: lstocchi <lstocchi@redhat.com>

Update packages/frontend/src/pages/ImportModels.svelte

80fc06b

Co-authored-by: axel7083 <42176370+axel7083@users.noreply.github.com> Signed-off-by: Luca Stocchi <49404737+lstocchi@users.noreply.github.com>

Update packages/frontend/src/pages/ImportModels.svelte

ad6c90a

Co-authored-by: axel7083 <42176370+axel7083@users.noreply.github.com> Signed-off-by: Luca Stocchi <49404737+lstocchi@users.noreply.github.com>

Update packages/frontend/src/lib/table/model/ModelColumnName.svelte

90bf758

Co-authored-by: axel7083 <42176370+axel7083@users.noreply.github.com> Signed-off-by: Luca Stocchi <49404737+lstocchi@users.noreply.github.com>

fix: fix format

8a20e4c

Signed-off-by: lstocchi <lstocchi@redhat.com>

fix: show N/A when memory value is not available

4d8069c

Signed-off-by: lstocchi <lstocchi@redhat.com>

fix: set memory equals to file size

d8ad8bb

Signed-off-by: lstocchi <lstocchi@redhat.com>

lstocchi force-pushed the i173 branch from 3a8b7ae to d8ad8bb Compare April 9, 2024 17:24

fix: fix tests

34b477b

Signed-off-by: lstocchi <lstocchi@redhat.com>

lstocchi force-pushed the i173 branch from a4471ea to 34b477b Compare April 10, 2024 10:23

jeffmaury approved these changes Apr 11, 2024

View reviewed changes

lstocchi merged commit af00eed into containers:main Apr 11, 2024
4 checks passed

lstocchi deleted the i173 branch April 11, 2024 07:33

feloy mentioned this pull request Apr 12, 2024

inconsistent UX when trying to start a recipe with a non downloaded model #757

Closed

axel7083 mentioned this pull request Apr 17, 2024

fix: add audio_to_text and object_detection recipes to the catalog #869

Merged

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: allow to import local models #771

feat: allow to import local models #771

lstocchi commented Apr 2, 2024 •

edited by feloy

slemeur commented Apr 2, 2024

lstocchi commented Apr 3, 2024

axel7083 left a comment

axel7083 Apr 3, 2024

lstocchi Apr 3, 2024

axel7083 Apr 3, 2024

lstocchi Apr 3, 2024

axel7083 Apr 3, 2024

lstocchi Apr 3, 2024

lstocchi Apr 3, 2024

axel7083 Apr 3, 2024

jeffmaury Apr 4, 2024

axel7083 left a comment •

edited

lstocchi commented Apr 3, 2024

Breadcrumb

Import timeout

Selected file

jeffmaury commented Apr 4, 2024

jeffmaury left a comment

lstocchi commented Apr 4, 2024

lstocchi commented Apr 4, 2024

feloy commented Apr 4, 2024

lstocchi commented Apr 4, 2024

jeffmaury commented Apr 4, 2024

axel7083 commented Apr 4, 2024 •

edited

vrothberg commented Apr 4, 2024

jeffmaury commented Apr 4, 2024

axel7083 left a comment

lstocchi commented Apr 9, 2024

jeffmaury left a comment

feat: allow to import local models #771

feat: allow to import local models #771

Conversation

lstocchi commented Apr 2, 2024 • edited by feloy

What does this PR do?

Screenshot / video of UI

What issues does this PR fix or reference?

How to test this PR?

slemeur commented Apr 2, 2024

lstocchi commented Apr 3, 2024

axel7083 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

axel7083 left a comment • edited

Choose a reason for hiding this comment

Breadcrumb

Import timeout

Selected file

lstocchi commented Apr 3, 2024

Breadcrumb

Import timeout

Selected file

jeffmaury commented Apr 4, 2024

jeffmaury left a comment

Choose a reason for hiding this comment

lstocchi commented Apr 4, 2024

lstocchi commented Apr 4, 2024

feloy commented Apr 4, 2024

lstocchi commented Apr 4, 2024

jeffmaury commented Apr 4, 2024

axel7083 commented Apr 4, 2024 • edited

vrothberg commented Apr 4, 2024

jeffmaury commented Apr 4, 2024

axel7083 left a comment

Choose a reason for hiding this comment

lstocchi commented Apr 9, 2024

jeffmaury left a comment

Choose a reason for hiding this comment

lstocchi commented Apr 2, 2024 •

edited by feloy

axel7083 left a comment •

edited

axel7083 commented Apr 4, 2024 •

edited