7
\$\begingroup\$

For a project I am using the yelp dataset (found here: https://www.yelp.com/dataset) to create a Hashset of all verbs, nouns and adjectives found in the restaurant reviews. I have it up and running using the stanford nlp pipeline, however it is quite slow (takes about 1 hour to process 10000 reviews) and the dataset contains a few million reviews. I am not an advanced programmer, I usually barely get it working so I really need help increasing the performance of my program. General coding advice is very much appreciated as well!

My code is as structured as follows: I have a MyCorpus class that has a function review_loader(). This function loads one review ( a json object) and puts the relevant data in a class named review. review contains a function that performs the pipeline operation and returns all nouns, verbs and adjectives of the review as a HashSet, I then add this hashset to a global hashset which will contain all nouns, verbs and adjectives of the yelp dataset.

Code for the relevant functions can be seen below:

Review.java

public class review {
private  String text;
private String business_id;
private int stars;
private ArrayList<String> listOfSentences = new ArrayList<String>();
private ArrayList<String> pos_tags = new ArrayList<String>();
private HashSet<String> all_terms = new HashSet<String>();

public review() {
}
public HashSet<String> find_terms(StanfordCoreNLP pipeline) {
CoreDocument doc = new CoreDocument(text);
pipeline.annotate(doc);
for(int f = 0; f <doc.sentences().size(); f++) {
    for (int d = 0; d < doc.sentences().get(f).tokens().size(); d++) {
        String tag = doc.sentences().get(f).posTags().get(d);
        CoreLabel word = doc.sentences().get(f).tokens().get(d);
        if (tag.contains("VB") == true|| tag.contains("JJ") == true || tag.contains("NN") == true);{
            String pattern ="[\\p{Punct}&&[^@',&]]";
            // Create a Pattern object
            Pattern r = Pattern.compile(pattern, Pattern.CASE_INSENSITIVE);
            // Now create matcher object.
            Matcher m = r.matcher(word.originalText());
            if (m.find() || word.originalText() == "") {

            } else {
               all_terms.add(word.originalText());
            }
        }

    }
}
return all_terms;


}

MyCorpus.java

public class MyCorpus{

private String filelocation_review;
private String filelocation_business;
private String filelocation_pos;
private ArrayList<String> restaurants = new ArrayList<String>();
private Set<String> allTerms = new HashSet<String>();


public MyCorpus(String filelocation_review, String filelocation_business, String filelocation_pos) {
    this.filelocation_review = filelocation_review;
    this.filelocation_business = filelocation_business;
    this.filelocation_pos = filelocation_pos;

}
    public void review_loader() throws FileNotFoundException, UnsupportedEncodingException {
    int counter = 0;
    Properties props = new Properties();
    // set the list of annotators to run
    props.setProperty("annotators", "tokenize,ssplit,pos,parse");
    // set a property for an annotator, in this case the coref annotator is being
    // set to use the neural algorithm
    props.setProperty("coref.algorithm", "neural");
    // build pipeline
    StanfordCoreNLP pipeline = new StanfordCoreNLP(props);

    MaxentTagger tagger = new MaxentTagger(filelocation_pos);
    InputStream is_r = new FileInputStream(filelocation_review);
    Reader r_r = new InputStreamReader(is_r, "UTF-8");
    Gson gson_r = new GsonBuilder().create();
    JsonStreamParser p = new JsonStreamParser(r_r);
    while (p.hasNext()) {
        counter += 1;
        JsonElement e = p.next();
        if (e.isJsonObject()) {
            review review = gson_r.fromJson(e, review.class);
            // This if statement checks if the review belongs to a restaurant by matching the business id to a list of all business_id's of a restaurant created previously
            if (restaurants.contains(review.get_id())) {
                HashSet<String> review_terms = review.find_terms(pipeline);
                allTerms.addAll(review_terms);
                System.out.println("size:" + allTerms.size() + "reviews processed: " + counter);            
            }
            }

        }
    public static void main(String args[]) throws IOException {
    // WHEN YOU RUN THE FILE CHANGE THE 3 FILELOCATIONS OF THE MYCORPUS CLASS!
    MyCorpus yelp_dataset = new MyCorpus("E:\\review.json", "E:\\business.json", "C:\\Users\\Ruben\\git\\Heracles\\stanford-postagger-2018-10-16\\models\\english-bidirectional-distsim.tagger");
    ArrayList<String> restaurants = yelp_dataset.business_identifier();
    yelp_dataset.review_loader();
    }

If there is anything that is unclear or seems weird, please do ask and thank you for taking the time to read this question.

Kind regards, Ruben

\$\endgroup\$
3
\$\begingroup\$

Adding to what @dariosicily mentioned already:

Performance

If you just need to find out a part-of-speech of each word and do not need to build a phrase-structure tree of sentences, you need only to specify 3 annotations (without parse):

props.setProperty("annotators", "tokenize,ssplit,pos");

I assume this can give you a significant boost in performance.

Since you are not doing coreference resolution, you don't need this line either:

props.setProperty("coref.algorithm", "neural");

Incorrect if-block

if (tag.contains("VB") == true|| tag.contains("JJ") == true || tag.contains("NN") == true);{
            String pattern ="[\\p{Punct}&&[^@',&]]";
...
}

You should remove the semicolon before the curly bracket, since currently, it terminates the if-block (and makes it empty), so the instructions inside the curly brackets will always be executed! The code above is now equal to the following:

if (tag.contains("VB") == true|| tag.contains("JJ") == true || tag.contains("NN") == true){
    /* Doing nothing */
}

{
    String pattern ="[\\p{Punct}&&[^@',&]]";
...
}

| improve this answer | |
\$\endgroup\$
  • 1
    \$\begingroup\$ Absolutely correct to remark about the semicolon, I have not added it because for me it was a typo due to a bad paste and copy, anyway in the doubt better to advise about it the OP. \$\endgroup\$ – dariosicily Apr 1 at 12:37
  • 1
    \$\begingroup\$ Thank you so much for answering! I didn't realize I didn't need those functions, those took 80% of all run time of the program! I was now able to run the program within ~4 hours. \$\endgroup\$ – Ruben Eschauzier Apr 2 at 9:35
4
\$\begingroup\$

Welcome to Code Review, here some suggestions about your code:

public class review { ... }

Java classnames always begin with uppercase letter so rename it to Review.

private ArrayList<String> restaurants = new ArrayList<String>();

In java language it is preferable using if possible an interface like List on the left part of the assignment so if you change the concrete class implementing the interface you don't notice the change in your code like below:

private List<String> restaurants = new ArrayList<String>();

Same approach from returning value from method:

public HashSet<String> find_terms(StanfordCoreNLP pipeline) { ... }

Use instead:

public Set<String> find_terms(StanfordCoreNLP pipeline) { ... }

You have this method and doc.sentences() seems me a List:

for(int f = 0; f <doc.sentences().size(); f++) {
    for (int d = 0; d < doc.sentences().get(f).tokens().size(); d++) {
        String tag = doc.sentences().get(f).posTags().get(d);
        CoreLabel word = doc.sentences().get(f).tokens().get(d);
        if (tag.contains("VB") == true|| tag.contains("JJ") == true || tag.contains("NN") == true);{
           String pattern ="[\\p{Punct}&&[^@',&]]";
           // Create a Pattern object
           Pattern r = Pattern.compile(pattern, Pattern.CASE_INSENSITIVE);
           // Now create matcher object.
           Matcher m = r.matcher(word.originalText());
           if (m.find() || word.originalText() == "") {
           } else {
           all_terms.add(word.originalText());
        }
    }
}

Rewrite it in a more readable way:

for(Sentence sentence : doc.sentences()) {
    for (int d = 0; d < sentence.token.size(); ++d) {
        String tag = sentence.posTags().get(d);
        CoreLabel word = sentence.tokens().get(d);
        //other instructions
    }
}

The boolean condition:

tag.contains("VB") == true|| tag.contains("JJ") == true || tag.contains("NN") == true

You can rewrite it like this:

tag.contains("VB") || tag.contains("JJ") || tag.contains("NN") 

Your pattern:

String pattern ="[\\p{Punct}&&[^@',&]]";
Pattern r = Pattern.compile(pattern, Pattern.CASE_INSENSITIVE);

You are calculating it for every iteration of the loop, put it outside your loop:

String pattern ="[\\p{Punct}&&[^@',&]]";
Pattern r = Pattern.compile(pattern, Pattern.CASE_INSENSITIVE);
for(Sentence sentence : doc.sentences()) {
    for (int d : sentence.token.size()) {
        String tag = sentence.posTags().get(d);
        CoreLabel word = sentence.tokens().get(d);
        //other instructions
    }
}

The if else you are using:

if (m.find() || word.originalText() == "") {
} else {
   all_terms.add(word.originalText());
}

You are doing an error here using the operator == and not the equals method for comparing strings; rewrite the method like this:

if (!m.find() && !word.originalText().equals("")) {
   all_terms.add(word.originalText());
}
| improve this answer | |
\$\endgroup\$
  • 2
    \$\begingroup\$ What is for (int d : sentence.token.size())? Did they introduce a foreach integer up to integer in one of the last relases? \$\endgroup\$ – mtj Apr 1 at 11:04
  • 1
    \$\begingroup\$ @mtj Thanks , I wrote a stupid thing and I fix it. \$\endgroup\$ – dariosicily Apr 1 at 11:58
  • \$\begingroup\$ Thank you so much for answering, I incorporated your and the other commenters notes and it works like a charm! I really appreciate the general notes for better code \$\endgroup\$ – Ruben Eschauzier Apr 2 at 9:33
  • \$\begingroup\$ @RubenEschauzier You are welcome, if you have other questions post them on the Code Review site. \$\endgroup\$ – dariosicily Apr 2 at 15:13

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.