Skip to content
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

ML endpoints #1

Open
wants to merge 25 commits into
base: main
Choose a base branch
from
Open

ML endpoints #1

wants to merge 25 commits into from

Conversation

DiegoPino
Copy link
Member

What?

@alliomeria I will request your review on the Python code here. I am aware the new code is not perfect and it will require me to explain what is happening before you can review. But your new Python skills/formal learning are required/needed

Let's talk about this tomorrow (Friday 21st, Summer) when you have a time. Thanks

Download the medium model to the models folder for testing
wget https://github.com/ultralytics/assets/releases/download/v8.2.0/yolov8m.pt
@alliomeria yolov8 as service. WIP but works
means via docker-compose ENVs
Of course you want to mount /models so you can deploy from the host
… On L1

affecting score and who else knows what when searching with Solr
While testing because i have two Servers running i change to 6401. Then fetch and fail. gosh
… correctly

And we are doing research so i need all options available. ML processors will also get an option to define the requested norm.
This is above of my pay grade ...https://github.com/FlagOpen/FlagEmbedding
see bge-small-en-v1.5
So far i am letting hugging face download it on first call. Not ideal ... so the
app.config["SENTENCE_TRANSFORMER_MODEL_FOLDER"]  has no use yet.
But once i move into building docker i will fetch the folder with requires LFS access via git (more more bytes/apps, etc. Bananas)
For now i am letting the app to download the model on a first run but will eventually manage to get it upfront. I just don't want a huge Docker Container
There is no communication going out to the world but i am not returning Gender or Age (Unethical and also Spookie )
This model is the same one used internally by Apple (ArcFace)
I need to figure out (at the Archipelago side) how to make a single Processor generate multiple Flavor Documents, so right now i am extracting embbeding  (l2) vector for only ONE face, the one with the highest score and normalizing the bounding box @alliomeria (just pinging bc this is the lonely ML mountain and there is no queen/king/dwarf under the mountain, not even a dragon)
@DiegoPino DiegoPino self-assigned this Jun 20, 2024
Copy link

@alliomeria alliomeria left a comment

Choose a reason for hiding this comment

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

Ok @DiegoPino, went through a first pass review and left some comments. I have a couple other comments & questions for you, will save for our live review later today. This is a lot of great work on your part, hats off to you! 🎩 Let's talk more later about all this!

if not params['norm']:
params['norm'] = 'l2'

if params['norm'] and params['norm'] not in ['l1','l2','max']:

Choose a reason for hiding this comment

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

l1 not used anywhere? (seeing notes below)

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes not used. Reason I wanted to leave/show that for you bc the comparison for image ML models when using one or the other is semantically so so different. Let's talk about that. I can also use it and then let the SBR processors just call L2.

# Vector size for this layer (i think by default it will be numlayers - 2 so 20) is 576
# array.reshape(-1, 1) if your data has a single feature or array.reshape(1, -1) if it contains a single sample
# This "should" return a Unit Vector so we can use "dot_product" in Solr
# Even if Norm L1 is better for comparison, dot product on Solr gives me less than 1 of itself. So will try with L2

Choose a reason for hiding this comment

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

👍

img = loadImage(params['iiif_image_url'], 640)
if img is not False:
app = FaceAnalysis(providers=['CPUExecutionProvider'])
# This will get all models. Inclussive age, gender. (bad juju) etc. But we won't return those

Choose a reason for hiding this comment

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

Good that we're not returning these. It would be better if there was a way to prevent these factors from being processed at all, but I understand that might not be possible. Maybe a good reason to consider not using insightface at all?

Copy link
Member Author

Choose a reason for hiding this comment

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

Hi, you are right but the reason I can not avoid it is the Embedding (for the vector/just image comparison), that one is not generated when I do not run Analysis.

if img is not False:
app = FaceAnalysis(providers=['CPUExecutionProvider'])
# This will get all models. Inclussive age, gender. (bad juju) etc. But we won't return those
# We could limit to just 'detection' and 'recognition' (last one provides the embeddings)

Choose a reason for hiding this comment

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

🤔

data['message'] = 'Sentence transformer (embedding)'
data['sentence_transformer'] = {}
params = {}
# For s2p(short query to long passage) retrieval task, each short query should start with an instruction (instructions see Model List...NOTE link leads to nothing good). But the instruction is not needed for passages. #

Choose a reason for hiding this comment

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

🆗

nlpserver.py Outdated
@@ -12,7 +12,7 @@
# configurations
#app.config['var1'] = 'test'
app.config["YOLO_MODEL_NAME"] = "yolov8m.pt"
app.config["MOBILENET_MODEL_NAME"] = "mobilenet_v3_small.tflite"
app.config["MOBILENET_MODEL_NAME"] = "mobilenet_v3_large.tflite"

Choose a reason for hiding this comment

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

👍

Copy link
Member Author

Choose a reason for hiding this comment

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

Really a matter of space/taste/ The small one really had little inference capabilities. But I will defer to you on this

…hts)

and set a root/download folder for insightface, which will be pre-loaded by the docker container
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