Gravitate-Health / lens-selector-mvp2

Set of lenses for MVP2
Apache License 2.0
0 stars 0 forks source link

[LNS] External Lenses support #14

Closed joofio closed 4 months ago

amedranogil commented 9 months ago

provide an example of Lens that calls an external http service

amedranogil commented 9 months ago

"Proxy lens": a lens that off loads workload to a service. This will allow to develop lenses in other languajes.

amedranogil commented 9 months ago

might be a good idea to categorize lenses on their external services needs:

  1. offline: no data is sent outside
  2. private online: the lens connects to external services, but no sensitive data is shared outside (this might incur in some privacy exploitation by which observer of the connections could determine at least this lens is being executed)
  3. full online: sensitive data is shared with external services

This may need to be included in https://github.com/hl7-eu/gravitate-health/issues/29

aalonsolopez commented 9 months ago

Hello all! A quick update on this task. This lens is quite complex, because I'm developing an example of lens made on Python, which you can find in https://github.com/Gravitate-Health/external-lens-service-python. This is going to be an "exact"copy of the Pregnancy Lense, but in other programming language and on an "external service" (on the real life example it will be external, but for now this service will be hosted inside the cluster). Is there anything that I should have in mind before fully develop and test this?

joofio commented 9 months ago

well my idea is to work as proxy. Like you receive the input, validate safety and authorization and pass it on? something like it?

aalonsolopez commented 9 months ago

Ok, so I should think about how I will make the "safety and authorization" step, because now it's obvious for us because is our service but... Maybe the authorization should be done for every lense and not only this. We don't have a way to know if an attacker could make it to execute the code that he/she wants and bypass even a normal lense. Maybe we should discuss how can we prove that a lense is executed instead of malicious software?

aalonsolopez commented 8 months ago

Lense is done and can be tested on Dev using the lens "lens-selector-mvp2_proxy-pregnancy". PR ongoing

joofio commented 7 months ago

Can we do this for the summary lens (https://github.com/joofio/summary-lens) ? what would the requirements be?

amedranogil commented 7 months ago

Yes. My only issue is with part 1 of the prompt, which would probably be the result of another LLM application to summarize from the IPS to something similar to the expected input from the LLM.

Another aspect to consider could be the modfile, system-level prompt, indicating how the LLM should overall behaviour, although this might be more applicable to the chatbot. we are considering ollama as solution to run the LLMs; it seems to be simple enough to deploy and manage, providing an unified API to the models. We only need to provide the infrastructure.

joofio commented 7 months ago

i dont summarize the IPS, i just take information from it to target the ePI summary. I also have the system level prompt here (which can be tweaked of course): https://github.com/joofio/summary-lens/blob/05a65f31835cfe59a274b61f1aeae671efb0a080/lens_app/core.py#L147

Those solutions seem fine but are to be set under the "lensing" correct? So i can also add them to manage more models . Great!

However my question is: how can we add this repo (we can work on it during that) to the FOSPS ? Probably as external lenses(?)

joofio commented 7 months ago

15.2.2024: Joao to create docker for lensing. EDIT: [DONE]

joofio commented 7 months ago

@aalonsolopez like discussed today, made the docker of the lens. jfcal/gh-summary-lens. needs 3 variables as stated in the readme. Let me know what else do you need.

joofio commented 7 months ago

@aalonsolopez updates on this?

aalonsolopez commented 7 months ago

Sorry, I forgot this, to be honest. Between this evening and tomorrow, I expect to have it done

aalonsolopez commented 7 months ago

@joofio just to have it clear,,, Is OPENAI_KEY necessary? Every call to OpenAI's API have a cost, unless we have it covered on the project...

joofio commented 7 months ago

it should be mandatory as a env parameter but we can add a dummy one. For other model, the URL is mandatory (ollama based)

aalonsolopez commented 7 months ago

Perfect!

aalonsolopez commented 7 months ago

Just for you to know @joofio, I was trying to do something like you did in my free time, and I even built this tool (still WIP) to extract all the existent SNOMED codes and training an AI with them, to make a preprocessor PoC with a LLM. IDK if we can pass all the codes as context on a Modelfile in Ollama

aalonsolopez commented 6 months ago

The external lens (proxy lens) is working on dev... Should we close this issue?

joofio commented 6 months ago

its just the summary? didnt we had another?how can i test?

aalonsolopez commented 6 months ago

Try this :)

POST https://gravitate-health.lst.tfo.upm.es/bundlepackageleaflet-es-94a96e39cfdcd8b378d12dd4063065f9?preprocessors=preprocessing-service-manual&lenses=lens-selector-mvp2_proxy-pregnancy&patientIdentifier=alicia-1

joofio commented 5 months ago
HTTP/1.1 404 Not Found
date: Sat, 13 Apr 2024 14:09:20 GMT
server: istio-envoy
connection: close
content-length: 0

do i need to add anything else?

aalonsolopez commented 5 months ago

Wait the URL is totally wrong omg.

This is the good one https://gravitate-health.lst.tfo.upm.es/focusing/focus/bundlepackageleaflet-es-94a96e39cfdcd8b378d12dd4063065f9?preprocessors=preprocessing-service-manual&lenses=lens-selector-mvp2_proxy-pregnancy&patientIdentifier=alicia-1

joofio commented 5 months ago

nice it works! and the summary?

aalonsolopez commented 5 months ago

Summary lense is not ready yet as I let it paused due to some reasons, like the format it displays. But for sure I can get it for today or tomorrow. There is still time to refine it.

aalonsolopez commented 5 months ago

@joofio so the Summary Text should be inserted on the bottom side of the whole ePI (for example)?

joofio commented 5 months ago

no. right now i just need the summary alone. we can discuss further options later

aalonsolopez commented 5 months ago

oh the the job is done, not as a lens as we know but its done :) Right now I'm solving an error that the summary service raises which is pretty weird because seems to be a connection error, so for today's meeting I expect to have it solved

aalonsolopez commented 5 months ago

http://gravitate-health.lst.tfo.upm.es/ai/summary/bundlepackageleaflet-es-da0fc2395ce219262dfd4f0c9a9f72e1?preprocessors=preprocessing-service-manual&patientIdentifier=Cecilia-1&lenses=lens-summary-2&model=graviting-llama

@joofio

joofio commented 5 months ago

something wrong with language it seems?

joofio commented 5 months ago

this is llama? tried with mistral?

aalonsolopez commented 5 months ago
   Apparently, I'm working on trying to make a fix 

   On martes, abr 16, 2024 at 1:36 p. m., João Almeida ***@***.***> wrote: 

  something wrong with language it seems? 
  —Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you were mentioned.Message ID: ***@***.***> 
  ***@***.***": ***@***.***": "EmailMessage","potentialAction": ***@***.***": "ViewAction","target": "https://github.com/Gravitate-Health/lens-selector-mvp2/issues/14#issuecomment-2058880415","url": "https://github.com/Gravitate-Health/lens-selector-mvp2/issues/14#issuecomment-2058880415","name": "View Issue"},"description": "View this Issue on GitHub","publisher": ***@***.***": "Organization","name": "GitHub","url": "https://github.com"}}]
aalonsolopez commented 5 months ago
  This is a Medical Oriented derivation of LLaMa 

   -- 
  Sent from Canary 

   On martes, abr 16, 2024 at 1:37 p. m., João Almeida ***@***.***> wrote: 

  this is llama? tried with mistral? 
  —Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you were mentioned.Message ID: ***@***.***> 
  ***@***.***": ***@***.***": "EmailMessage","potentialAction": ***@***.***": "ViewAction","target": "https://github.com/Gravitate-Health/lens-selector-mvp2/issues/14#issuecomment-2058881083","url": "https://github.com/Gravitate-Health/lens-selector-mvp2/issues/14#issuecomment-2058881083","name": "View Issue"},"description": "View this Issue on GitHub","publisher": ***@***.***": "Organization","name": "GitHub","url": "https://github.com"}}]
joofio commented 5 months ago
  1. ~documentation~ (to be enhanced, but for the future)
  2. data for summary without personal data
  3. create this one with same syntax as the others lenses
aalonsolopez commented 5 months ago

we can do it on the same service, with separated lenses, or like the other lenses in Lens-Selector (a service that is going to eventually be removed). What are your preferences on this?

aalonsolopez commented 5 months ago

@jkiddo @joofio there is a new section on swagger, https://gravitate-health.lst.tfo.upm.es/swagger-fosps/?urls.primaryName=AI%20Services#/Lenses/get_ai_summary, but I remember it's not finished yet and there's a lot of things to talk :)

jkiddo commented 5 months ago

The text says its for "Request for a summary of an ePI using LLM AI". Why would that need patient as a reference?

aalonsolopez commented 5 months ago
  Because is what it does at this moment. As we have talked, the lenses with AI will have 2 versions, but the Swagger describes as it is now. 

   On martes, abr 16, 2024 at 4:32 p. m., Jens Kristian Villadsen ***@***.***> wrote: 

  The text says its for "Request for a summary of an ePI using LLM AI". Why would that need patient as a reference? 
  —Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you were mentioned.Message ID: ***@***.***> 
  ***@***.***": ***@***.***": "EmailMessage","potentialAction": ***@***.***": "ViewAction","target": "https://github.com/Gravitate-Health/lens-selector-mvp2/issues/14#issuecomment-2059240790","url": "https://github.com/Gravitate-Health/lens-selector-mvp2/issues/14#issuecomment-2059240790","name": "View Issue"},"description": "View this Issue on GitHub","publisher": ***@***.***": "Organization","name": "GitHub","url": "https://github.com"}}]
jkiddo commented 5 months ago

That does not answer 'why'?

joofio commented 5 months ago

The why is to use information about the patient to tailor the summary (like age )

jkiddo commented 5 months ago

What else does it use besides age?

joofio commented 5 months ago

Medication and condition

jkiddo commented 5 months ago

Then it should have a subsetted IPS. In terms of data minimization it should only handle what it needs

joofio commented 5 months ago

Makes sense. But let's try to test it as is to check for errors and then we can improve it based on the feedback.

joofio commented 5 months ago

@aalonsolopez the language stuff is done right? can i test it?

aalonsolopez commented 5 months ago

You can test it. I think it's solved because the last time I used it it worked with both English and Spanish (not tested with Danish)

jkiddo commented 5 months ago

I cannot test it before it takes the IPS in e.g. the body.

aalonsolopez commented 5 months ago

I know. For next week I will try to have the first version that does not need the IPS, as we said on Tuesday's meeting

jkiddo commented 5 months ago

What does it then take as argument?

aalonsolopez commented 5 months ago

What does it then take as argument?

It takes the medicine, the ePI Identifier and the PatientIdentifier (for now)