Skip to content
This repository has been archived by the owner on Nov 3, 2023. It is now read-only.

Flask implementation #4322

Closed
wants to merge 23 commits into from
Closed

Conversation

khushaljethava
Copy link
Contributor

Patch description
Many people in the community were asking for the Parlai implementation on the flask framework. So I have created a script to run parlai models on the Flask framework and make flask API for the parlai model.

Testing steps

Other information

I haven't changed or modified any code related to the pariai I have just added two files one with flask code and one readme file for the same. also, I have added path the flask API code in the main README.me file.

khushaljethava and others added 2 commits January 25, 2022 23:30
Added folder with name flask_api_dome and added code to implement parlai model with flask framework and readme file in the same folder. Also added URL to flask API folder on main README.md file.
Copy link
Contributor

@stephenroller stephenroller left a comment

Choose a reason for hiding this comment

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

This is great! Thanks so much for adding!

I don't love it as a top level directory though. Some options:

  • Since this is mostly just an example that fits in a README, we could move it into the docs (just rename the README.md to docs/source/tutorial_flask.md and add tutorial_flask in tutorial.md)
  • Move all the documentation inline into the script, and move the script into the scripts folder. Add the relevant templates so that the docs generate it.

What do you think?

Some options:

  • Move it to scripts

@khushaljethava
Copy link
Contributor Author

  • tutorial_flask

Thank you so much for the response @stephenroller , I have done the necessary changes as per your suggestions. Please review It.

@khushaljethava
Copy link
Contributor Author

@stephenroller can you help me what I'm doing wrong here?

@mojtaba-komeili
Copy link
Contributor

Thanks @khushaljethava for adding this. Please make sure that your added scripts are consistent with the scripts that we have in the script folder. This is a short script to follow as a good example. Note the extension of ParlaiScript class and implementation of setup_args, and run methods.

__Authors__: Khushal Jethava


### Parl.ai model implement on flask framework
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: it is better to use as ParlAI instead of Parl.ai.


Just change the model path and model name inside the create_agent_from_model_file function.

```python
Copy link
Contributor

Choose a reason for hiding this comment

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

Please change this to be a parameter set while running the script. Add that in your setup_args implementation. The default can be the zoo model.

requirements.txt Outdated
@@ -1,3 +1,4 @@
flask
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it possible to avoid installing an extra web server? We already use Tornado web server, could you redo this in Tornado to make our ParlAI installation leaner? Thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mojtaba-komeili Thank you for the guidance, I have created a tornado script with Parlai setup_args but not help with the result still trying to improve it, I'll get back to you once it will be completed and any idea about the flask solution? @stephenroller

Copy link
Contributor

Choose a reason for hiding this comment

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

Flask is fine by me, and we don't need this imported for parlai flask_demo, at least not now.

Copy link
Contributor

@stephenroller stephenroller left a comment

Choose a reason for hiding this comment

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

looks like you got the right setup, and I'm fine with flask -- it's good for tutorial purposes. I suspect the website build is broken because of the global model import.

requirements.txt Outdated
@@ -1,3 +1,4 @@
flask
Copy link
Contributor

Choose a reason for hiding this comment

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

Flask is fine by me, and we don't need this imported for parlai flask_demo, at least not now.

app = Flask(__name__)

#import model from the model file can be pretrained or fine tuned
blender_agent = create_agent_from_model_file("zoo:blender/blender_90M/model")
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you move this initialization into main somehow, so that the website builder doesn't do this?

# the associated function.
@app.route("/response", methods=["GET","POST"]) # API URL
def chatbot_response(): # function name
data = request.json # Take input as json format
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: you'll need to format all the comments

Just change the model path and model name inside the create_agent_from_model_file function.

```python
blender_agent = create_agent_from_model_file("Model Name")
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
blender_agent = create_agent_from_model_file("Model Name")
blender_agent = create_agent_from_model_file("path_to_model_or_zoo_name")

@khushaljethava
Copy link
Contributor Author

Im not getting what im doing wrong here? @stephenroller

docs/source/tutorial_flask.md Outdated Show resolved Hide resolved
parlai/scripts/flask_demo.py Outdated Show resolved Hide resolved
docs/source/tutorial_flask.md Outdated Show resolved Hide resolved
docs/source/tutorial_flask.md Outdated Show resolved Hide resolved
parlai/scripts/flask_demo.py Outdated Show resolved Hide resolved
Copy link
Contributor Author

@khushaljethava khushaljethava left a comment

Choose a reason for hiding this comment

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

Looks perfect

@stephenroller
Copy link
Contributor

Yup, this is all good. I got pulled away to try to fix the tests in another PR.

@stephenroller stephenroller mentioned this pull request Mar 21, 2022
@stephenroller
Copy link
Contributor

Completed in #4433.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants