deepset-ai / COVID-QA

API & Webapp to answer questions about COVID-19. Using NLP (Question Answering) and trusted data sources.
Apache License 2.0
343 stars 119 forks source link

Feature : Matched Question and Feedback Option #65

Closed theapache64 closed 4 years ago

theapache64 commented 4 years ago

Thanks again for creating this PR. Great work!

Two comments:

Would be great to hear your thoughts on that and maybe address them in a separate PR.

Originally posted by @tholor in https://github.com/deepset-ai/COVID-QA/pull/58#issuecomment-602513354

theapache64 commented 4 years ago

@tholor

I've updated the reply format to

$emoji Answer Confidence : $confidence% 
Q: ${ans.question}
A: ${ans.answer}
🌍 Source : <a href="${ans.meta.link}">${ans.meta.source}</a>

Is this okay ?

tholor commented 4 years ago

Yes, perfect!

theapache64 commented 4 years ago

one more format

Q: ${ans.question}
A: ${ans.answer}
💪 Answer Confidence : $confidence% $emoji 
🌍 Source : <a href="${ans.meta.link}">${ans.meta.source}</a>

Which one is better? first or this?

theapache64 commented 4 years ago

Can u DM me at http://t.me/theapache64

theapache64 commented 4 years ago

@tholor When I try to post the feedback, am getting

{"detail":[{"loc":["body","request","questions"],"msg":"field required","type":"value_error.missing"}]}

from the server.

The swagger doc says feedback API needs 4 params.

{
  "question": "string",
  "answer": "string",
  "feedback": "relevant",
  "document_id": 0
}

but the python script only handles one.

I won't able to pass question and answer to the route, as Telegram only supports max 64bytes of data via callback_data. See here

tholor commented 4 years ago

@theapache64 yes, the API requires all four fields at the moment (and they are used in the python code you linked). Please also note that the value for "question" must be the one asked by the user and not the one we matched in the FAQs.

I see your limitations with 64bytes and can think of two solutions: 1) short term: we make "answer" an optional field as we could get this also via the document_id. Could you test if you already fulfill the 64 bytes requirement with this? 2) mid term: if this is still more than 64bytes, we could probably introduce a unique request_id and then reduce the request to {"request_id": 123, "feedback": "relevant"}. However, for this, we need quite some changes in our backend and the indexing in elastic.

theapache64 commented 4 years ago

Perfect. For now, we can go with the short term solution, but if the question is lengthy (more than 64bytes), the feedback won't be posted. I'd suggest you work on the midterm solution ASAP. Otherwise, we may lose some valuable feedback. What do you say?

and How much time do you need to make the answer variable optional?

tholor commented 4 years ago

How much time do you need to make the answer variable optional?

Implemented in #69. We can deploy it in the next hours. Will let you know once that's done.

I'd suggest you work on the midterm solution ASAP

Yes, we will tackle that. Can't promise a timeline though as most of us work in their regular jobs during the week.

tholor commented 4 years ago

@theapache64 latest master is deployed now (incl. the optional answer field)

theapache64 commented 4 years ago

image

It's done! Any changes? @tholor

tholor commented 4 years ago

Looking great! Thank you @theapache64