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

Make movie reviews model easier to run #34

Closed
loostrum opened this issue Sep 2, 2021 · 5 comments
Closed

Make movie reviews model easier to run #34

loostrum opened this issue Sep 2, 2021 · 5 comments
Assignees

Comments

@loostrum
Copy link
Member

loostrum commented Sep 2, 2021

Currently it needs quite a lof of code to run; most of that can be made part of the model itself.
This would allow for easy use of the model in tests as well

@loostrum loostrum assigned loostrum and unassigned loostrum Sep 2, 2021
@loostrum
Copy link
Member Author

loostrum commented Sep 2, 2021

Leaving as-is for now, we should talk to an NLP expert first to see how models are normally used.

@loostrum loostrum transferred this issue from dianna-ai/dianna-exploration Oct 19, 2021
@loostrum loostrum self-assigned this Jan 10, 2022
@loostrum
Copy link
Member Author

loostrum commented Jan 10, 2022

The input to the model should remain text, but the functions to create tokens from the input string will be made part of the model. The goal is to avoid having to define a lot of boilerplate code in the dianna repository just to be able to run the model.

@loostrum
Copy link
Member Author

Draft PR in the exploration repo: dianna-ai/dianna-exploration#147
The pytorch model is working, but after conversion to onnx it fails because there are no inputs.
Possibly related issue: pytorch/pytorch#44299

@loostrum loostrum added the standup Temp label- for disscussion with the team next standup label Jan 10, 2022
@loostrum loostrum removed the standup Temp label- for disscussion with the team next standup label Jan 13, 2022
@loostrum
Copy link
Member Author

We will not use the simplified model as it doesn't work. This issue can be closed when the simplifications that don't affect the onnx model itself are merged in dianna-exploration.

@elboyran
Copy link
Contributor

@loostrum could you, please, check if this can be closed?

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

No branches or pull requests

2 participants