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

feat(SentimentAnalyzer): Improve performance #3

Merged

Conversation

kamranayub
Copy link
Contributor

@kamranayub kamranayub commented Apr 6, 2021

Changes

Results

  • Before: I didn't even let it finish but estimate was 100 minutes 😅
  • After: ~16 seconds

Specs: core i7-3770k @ 3.5ghz, 32gb ram

@kamranayub kamranayub marked this pull request as ready for review April 6, 2021 03:54
@kamranayub
Copy link
Contributor Author

Thanks for a simple library @arafattehsin! I was wondering what data model this is trained on? There is a list here of ones that could be used for other options:

https://kavita-ganesan.com/user-review-datasets/

Copy link
Owner

@arafattehsin arafattehsin left a comment

Choose a reason for hiding this comment

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

Thanks @kamranayub for such a great improvement. I definitely missed this one. I think I used Yelp Dataset to train this along with some other (which I can't recall) but it was picked up from Kaggle.

From your pointer, I just thought that we could also bring all of the reviews in one library but from different models. I have a strategy for that in my mind which I will shortly write in an issue. Please feel free to chip in your thoughts.

@arafattehsin arafattehsin merged commit 9d4d32e into arafattehsin:master Apr 6, 2021
@kamranayub kamranayub deleted the feat/perf-sentiment-analysis branch April 6, 2021 23:20
{
public static SentimentPrediction Predict(string text)
private static readonly Sentiments instance = new Sentiments();
Copy link

Choose a reason for hiding this comment

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

This needs to be [ThreadStatic] and initialized once per thread

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, addressed in #5

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.

3 participants