Preload / Prewarm custom models via URL on startup #475

vicilliar · 2023-05-12T10:55:13Z

What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
Feature
What is the current behavior? (You can also link to an open issue here)
Currently, the env var only accepts strings.
What is the new behavior (if this is a feature change)?
This allows the env var MARQO_MODELS_TO_PRELOAD to accept custom models as dicts with model and model_properties. A sample declaration would look like this:

export MARQO_MODELS_TO_PRELOAD='[
        "ViT-L/14",
         {
            "model": "generic-clip-test-model-2",
            "model_properties": {
                "name": "ViT-B/32",
                "dimensions": 512,
                "type": "clip",
                "url": "https://openaipublic.azureedge.net/clip/models/40d365715913c9da98579312b702a82c18be219cc2a73407c4526f58eba950af/ViT-B-32.pt"
            }
        }
    ]'

Does this PR introduce a breaking change? (What changes might users need to make in their application due to this PR?)
No
Have unit tests been run against this PR? (Has there also been any additional testing?)
Manual testing. Application tests will be made once the env var customization feature in Marqo API tests finishes
Related documentation changes (link commit/PR here)
To follow
Please check if the PR fulfills these requirements

The commit message follows our guidelines
Tests for the changes have been added (for bug fixes/features)
Docs have been added / updated (for bug fixes / features)

vicilliar · 2023-05-12T10:56:01Z

Manual testing matrix (OPEN CLIP and CLIP only so far)

Please let me know if there are other important edge cases to test

vicilliar · 2023-05-12T10:56:45Z

Models used for said manual testing:
OPEN CLIP

{
            "model": "random-open-clip-1",
            "model_properties": {
                "name": "ViT-B-32-quickgelu",
                "dimensions": 512,
                "url": "https://github.com/mlfoundations/open_clip/releases/download/v0.2-weights/vit_b_32-quickgelu-laion400m_avg-8a00ab3c.pt",
                "type": "open_clip"
            }
        }

CLIP

{
            "model": "generic-clip-test-model-2",
            "model_properties": {
            	"name": "ViT-B/32",
                "dimensions": 512,
                "url": "https://openaipublic.azureedge.net/clip/models/40d365715913c9da98579312b702a82c18be219cc2a73407c4526f58eba950af/ViT-B-32.pt",
                "type": "clip"
            }
        }

src/marqo/tensor_search/on_start_script.py

merging main

pandu-k · 2023-05-16T04:30:22Z

Thanks for the testing matrix!

Is the no "name" behaviour the same as when creating an index with this model normally? What is the expected behaviour @wanliAlex ?

wanliAlex · 2023-05-16T04:38:12Z

Thanks for the testing matrix!

Is the no "name" behaviour the same as when creating an index with this model normally? What is the expected behaviour @wanliAlex ?

Yes, because "name" is not required for loading clip models, but we make it a required filed in model properties.

wanliAlex · 2023-05-16T04:39:22Z

Do we want to hold this PR for a while until my custom HF models are merged? Do we want to test the custom HF models prewarming?

wanliAlex · 2023-05-16T04:41:50Z

The name check is here

But note that the "name" is not used for clip models if a URL is provided.

vicilliar · 2023-05-16T04:44:19Z

Do we want to hold this PR for a while until my custom HF models are merged? Do we want to test the custom HF models prewarming?

I'm under the impression this URL pre-warming feature is high priority, so it can be merged asap, but I'll need confirmation from @pandu-k

If ever, do we see the format for HF/S3 models being much different? Or are they also in the form model and model_properties?

wanliAlex · 2023-05-16T04:46:21Z

Do we want to hold this PR for a while until my custom HF models are merged? Do we want to test the custom HF models prewarming?

I'm under the impression this URL pre-warming feature is high priority, so it can be merged asap, but I'll need confirmation from @pandu-k

If ever, do we see the format for HF/S3 models being much different? Or are they also in the form model and model_properties?

Yeah they are similar. Feel free to merge if it's urgent as this is for production use.

pandu-k · 2023-05-16T04:49:48Z

Let's get this merged in first, so we can spin up a testing tag

wanliAlex · 2023-05-16T04:51:47Z

The PR looks good to me. Just to confirm, did you do the manual test on a dockerised marqo? @vicilliar

vicilliar · 2023-05-16T04:54:02Z

Ah good idea, i have not yet. Will do so

vicilliar · 2023-05-16T04:54:20Z

I did the tests on a local running marqo

wanliAlex · 2023-05-16T04:56:14Z

I did the tests on a local running marqo

Can you try to run several manual tests on a dockerised marqo and upload some screenshots? Would this be enough to test the production environment? @pandu-k

pandu-k · 2023-05-16T04:57:39Z

That should be pretty good. Try expected failure and expected successful cases. Ensure that search/add docs works as expected. The testing matrix was super useful

wanliAlex · 2023-05-16T04:59:52Z

tests/tensor_search/test_on_start_script.py

+                return True
+        assert run()
+
+    # TODO: test bad/no names/URLS in end-to-end tests, as this logic is done in vectorise call



can we have one more test to ensure the prewarmed model will not need to be download/load again?

can we have one more test to ensure the prewarmed model will not need to be download/load again?

For both search and add_document.

this we can save for the API end-to-end test

pandu-k · 2023-05-16T05:29:30Z

unit tests: https://github.com/marqo-ai/marqo/actions/runs/4988240487 [passed]
API tests: https://github.com/marqo-ai/marqo/actions/runs/4988241204 [passed]

vicilliar · 2023-05-16T08:43:27Z

failing 1 unit test on local:
This is probably just an existing problem, unit tests through GH actions work just fine

marqo.errors.BadRequestError: BadRequestError: Problem vectorising query. Reason: Unable to load model=ViT-B/16 on device=cpu with normalization=True. If you are trying to load a custom model, please check that model_properties={'name': 'ViT-B/16', 'dimensions': 512, 'notes': 'CLIP ViT-B/16', 'type': 'clip'} is correct and Marqo has access to the weights file.

============================================== short test summary info ==============================================
FAILED tests/tensor_search/test_add_documents.py::TestAddDocuments::test_mappings_arent_updated_images - marqo.errors.BadRequestError: BadRequestError: Problem vectorising query. Reason: Unable to load model=ViT-B/16 ...

vicilliar · 2023-05-16T11:13:07Z

Dockerized manual marqo testing:
works as expected (open clip).

Startup loads the models properly (hf model and a custom open clip model)
Upon indexing, no preloading is done (expected)
Search works fine, here is a sample query result for q="vegetable"

{'hits': [{'field_1': 'cabbage', '_id': '3', '_highlights': {'field_1': 'cabbage'}, '_score': 0.87822014}, {'field_1': 'red', '_id': '2', '_highlights': {'field_1': 'red'}, '_score': 0.8071192}, {'field_1': 'hello', '_id': '1', '_highlights': {'field_1': 'hello'}, '_score': 0.80202985}], 'query': 'vegetable', 'limit': 10, 'offset': 0, 'processingTimeMs': 284}

Here is the indexing/searching script used

import marqo
mq = marqo.Client()
settings = {
    "index_defaults": {
        "treat_urls_and_pointers_as_images": True,
        "model": 'generic-clip-test-model-1',
        "model_properties": {
            "name": "ViT-B-32-quickgelu",
                "dimensions": 512,
                "url": "https://github.com/mlfoundations/open_clip/releases/download/v0.2-weights/vit_b_32-quickgelu-laion400m_avg-8a00ab3c.pt",
                "type": "open_clip"
            },
        "normalize_embeddings": True,
    },
}
try:
    mq.delete_index("my-own-clip")
except:
    pass

response = mq.create_index("my-own-clip", settings_dict=settings)

mq.index("my-own-clip").add_documents(
    [
        {"_id": "1", "field_1": "hello"}, 
        {"_id": "2", "field_1": "red"}, 
        {"_id": "3", "field_1": "cabbage"}, 
    ]
)
res = mq.index("my-own-clip").search("vegetable")
print("search results are: ")
print(res)

vicilliar · 2023-05-16T11:27:56Z

Failure cases are also as expected:

Malformed JSON result:

Missing model result:

Missing model_properties result:

Also there are some mistakes in the error message, so I'm pushing a small change, just changing the message to this:

…into joshua/prewarm-any-model merging remote changes

vicilliar added 3 commits May 12, 2023 14:58

initial draft work

02349b0

initial draft work

9b0939f

updated error messages and added try catch to warmup

cc92826

pandu-k reviewed May 12, 2023

View reviewed changes

src/marqo/tensor_search/on_start_script.py Outdated Show resolved Hide resolved

pandu-k requested a review from wanliAlex May 15, 2023 02:34

vicilliar added 5 commits May 15, 2023 17:27

made preload_model an outside function

9e169d2

draft test work

89a4ad9

fixed unit tests for preload function

2c32092

fixed debug messages

7d3bd18

Merge branch 'mainline' into joshua/prewarm-any-model

1f18268

merging main

vicilliar requested a review from pandu-k May 15, 2023 13:49

vicilliar changed the title ~~Prewarm custom models via URL on startup~~ Preload / Prewarm custom models via URL on startup May 15, 2023

Update version.py to 0.0.20

4a8f403

wanliAlex requested changes May 16, 2023

View reviewed changes

pandu-k temporarily deployed to marqo-test-suite May 16, 2023 05:30 — with GitHub Actions Inactive

vicilliar added 2 commits May 16, 2023 19:31

updated error message

0e4b73c

Merge branch 'joshua/prewarm-any-model' of github.com:marqo-ai/marqo …

e07147b

…into joshua/prewarm-any-model merging remote changes

wanliAlex approved these changes May 16, 2023

View reviewed changes

pandu-k merged commit fb8d22d into mainline May 16, 2023

pandu-k deleted the joshua/prewarm-any-model branch May 16, 2023 12:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Preload / Prewarm custom models via URL on startup #475

Preload / Prewarm custom models via URL on startup #475

vicilliar commented May 12, 2023 •

edited

Loading

vicilliar commented May 12, 2023

vicilliar commented May 12, 2023

pandu-k commented May 16, 2023

wanliAlex commented May 16, 2023

wanliAlex commented May 16, 2023

wanliAlex commented May 16, 2023

vicilliar commented May 16, 2023

wanliAlex commented May 16, 2023

pandu-k commented May 16, 2023

wanliAlex commented May 16, 2023

vicilliar commented May 16, 2023

vicilliar commented May 16, 2023

wanliAlex commented May 16, 2023

pandu-k commented May 16, 2023

wanliAlex May 16, 2023

wanliAlex May 16, 2023

vicilliar May 16, 2023

pandu-k commented May 16, 2023 •

edited

Loading

vicilliar commented May 16, 2023 •

edited

Loading

vicilliar commented May 16, 2023

vicilliar commented May 16, 2023 •

edited

Loading

Preload / Prewarm custom models via URL on startup #475

Preload / Prewarm custom models via URL on startup #475

Conversation

vicilliar commented May 12, 2023 • edited Loading

vicilliar commented May 12, 2023

vicilliar commented May 12, 2023

pandu-k commented May 16, 2023

wanliAlex commented May 16, 2023

wanliAlex commented May 16, 2023

wanliAlex commented May 16, 2023

vicilliar commented May 16, 2023

wanliAlex commented May 16, 2023

pandu-k commented May 16, 2023

wanliAlex commented May 16, 2023

vicilliar commented May 16, 2023

vicilliar commented May 16, 2023

wanliAlex commented May 16, 2023

pandu-k commented May 16, 2023

wanliAlex May 16, 2023

Choose a reason for hiding this comment

wanliAlex May 16, 2023

Choose a reason for hiding this comment

vicilliar May 16, 2023

Choose a reason for hiding this comment

pandu-k commented May 16, 2023 • edited Loading

vicilliar commented May 16, 2023 • edited Loading

vicilliar commented May 16, 2023

vicilliar commented May 16, 2023 • edited Loading

vicilliar commented May 12, 2023 •

edited

Loading

pandu-k commented May 16, 2023 •

edited

Loading

vicilliar commented May 16, 2023 •

edited

Loading

vicilliar commented May 16, 2023 •

edited

Loading