Add sentiment analysis capability for 68 more languages #9

c-w · 2017-06-07T15:07:44Z

In the past, we integrated with the Cognitive Services Text Analytics API to extract the sentiment from a text. When we started the integration, the API only had support for 4 languages. Now, there is support for 15 languages.† However, Fortis would like to support many more languages. This PR addresses that problem.

Most sentiment detection work is done for the big NLP languages (English, German, etc.), however, I did manage to find a paper by the data science group at Stony Brook University which focused on sentiment analysis work for all the other languages out there. Through a combination of a variety of techniques (including machine translation and morphological propagation across linguistically similar languages), they managed to create word-polarity lists for over 100 languages. I analyzed the lists they provide, kept the lists for the languages where they have at least 500 positive and negative terms and uploaded them to our fortis-models blob in a machine readable format.

Inside of Fortis, we then use the word polarity lists to compute sentiment for languages that are unsupported by Cognitive Services, like so:

Download the polarity lists from blob (cache on disk)
Read the positive/negative words (cache in memory)
Count how many positive/negative words are in the sentence.
If there are more positive words, assume positive sentiment. If there are more negative words, assume negative sentiment. If the count is the same, assume neutral sentiment.

This approach is super naive (I've asked for support from the Machine Learning TWG to improve the approach if possible), but it's better than nothing and similar to how we did sentiment analysis in Fortis-v1 so I assert it's good enough for now.

This closes Issue#18.

NB: The PR also includes some re-structuring that was necessary to implement the new functionality, e.g. moving around some packages, extracting helper utilities, etc.

†: Cognitive Services currently support English, Spanish, Portuguese, French, German, Italian, Dutch, Norwegian, Swedish, Polish, Danish, Finnish, Russian, Greek and Turkish.

This can be shared between the place recognizer and the file-based sentiment detector.

kevinhartman

LGTM w/ comments.

kevinhartman · 2017-06-08T14:55:44Z

...cala/com/microsoft/partnercatalyst/fortis/spark/transforms/sentiment/SentimentDetector.scala

    }
  }
 }
+
+object SentimentDetector {
+  val POSITIVE: Double = 1.0


Scala constants should be Pascal-cased.

Done in cb10a83

kevinhartman · 2017-06-08T15:06:24Z

src/main/scala/com/microsoft/partnercatalyst/fortis/spark/transforms/ZipModelsProvider.scala

-  enabledLanguages: Set[String] = Set("de", "en", "es", "eu", "it", "nl")
-) extends Serializable with Logger {
+class ZipModelsProvider(
+  formatModelsDownloadUrl: String => String,


Naming this like a function might make its use clearer. Maybe something like modelsUrlFromLanguage.

Done in 9ec3011

kevinhartman · 2017-06-08T15:12:46Z

src/main/scala/com/microsoft/partnercatalyst/fortis/spark/transforms/ZipModelsProvider.scala

-) extends Serializable with Logger {
+class ZipModelsProvider(
+  formatModelsDownloadUrl: String => String,
+  modelsSource: Option[String] = None


nit: how about modelDirectory?

It could also be a custom URL to a models zip file.

kevinhartman · 2017-06-08T15:17:13Z

.../scala/com/microsoft/partnercatalyst/fortis/spark/transforms/locations/PlaceRecognizer.scala

+
+      logDebug(s"Analyzed text $text in language $language: $kaf")
+
+      kaf.getEntities.toList.filter(entityIsPlace).map(_.getStr).toSet


toSet not needed here since return type is Iterable.

The function fetches the entities in a sentence so there shouldn't be duplicates. I've changed the return type to Set in 5861eab.

kevinhartman · 2017-06-08T15:29:55Z

.../scala/com/microsoft/partnercatalyst/fortis/spark/transforms/locations/PlaceRecognizer.scala

+
+      kaf.getEntities.toList.filter(entityIsPlace).map(_.getStr).toSet
+    } catch {
+      case ex @ (_ : NullPointerException | _ : IOError) =>


Which code will throw a null pointer exception? If it's OpeNER, we should likely be calling it differently, since throwing NullPointerException should not be part of an error handling contract.

I wish it weren't the case... but unfortunately the OpeNER code is research-quality code so it throws a NPE in some cases when it can't load a model since they swallow an IOException internally and then later hit the null-pointer.

Ah yikes. Can we tighten the Try scope to just the relevant OpeNER statements?

Done in e89573a.

kevinhartman · 2017-06-08T15:58:37Z

.../microsoft/partnercatalyst/fortis/spark/transforms/sentiment/WordListSentimentDetector.scala

+import scala.io.Source
+
+@SerialVersionUID(100L)
+class WordListSentimentDetector(


Perhaps you could define a trait for SentimentDetector.

There could be an interface for it, but right now there's only two implementations which are both wrapped by a single class so there wouldn't be much value in it.

I agree that it wouldn't provide immediate value, but might be nice to think through to set us up for easier extensibility in the future.

Done in c7008f8.

kevinhartman · 2017-06-08T16:05:05Z

.../microsoft/partnercatalyst/fortis/spark/transforms/sentiment/WordListSentimentDetector.scala

+
+import com.microsoft.partnercatalyst.fortis.spark.logging.Loggable
+import com.microsoft.partnercatalyst.fortis.spark.transforms.ZipModelsProvider
+import com.microsoft.partnercatalyst.fortis.spark.transforms.nlp.Tokenizer.tokenize


Importing a specific function like this globally seems a little misleading. If you change tokenize to apply in Tokenizer, then you can tokenize in this file with Tokenizer("input string")

Done in 880c5af

kevinhartman · 2017-06-08T16:06:21Z

src/main/scala/com/microsoft/partnercatalyst/fortis/spark/transforms/nlp/Tokenizer.scala

+object Tokenizer {
+  @transient private lazy val wordTokenizer = """\b""".r
+
+  def tokenize(sentence: String): Seq[String] = {


Might as well call this apply so we can treat Tokenizer like a global function.

Done in 880c5af

kevinhartman · 2017-06-08T16:08:24Z

.../microsoft/partnercatalyst/fortis/spark/transforms/sentiment/WordListSentimentDetector.scala

+  }
+
+  private def computeSentimentScore(numPositiveWords: Int, numNegativeWords: Int) = {
+    if (numPositiveWords > numNegativeWords) {


This would be much more beautiful with a match :)

I'm not a fan of sprinkling Scala constructs left right and center which make the code harder to understand for folks coming from more mainstream languages.

Looking at this again, I'd say it's fine as is, but as a general response, this codebase is written in Scala 😛.

Sure; then again, Java is as close to a lingua franca as we get in the software world so using Scala as Java++ (as opposed to going crazy and using Scala as a Haskell dialect) will make the code easier for others to grok :)

kevinhartman · 2017-06-08T16:23:01Z

...rosoft/partnercatalyst/fortis/spark/transforms/sentiment/WordListSentimentDetectorSpec.scala

+) extends WordListSentimentDetector {
+
+  protected override def readWords(path: String): Set[String] = {
+    if (path.contains("pos.txt")) {


nit: endsWith

Done in d5cbd60.

c-w added 14 commits June 7, 2017 15:15

Extract wrapper sentiment detector

3a89a58

Extract HasZipModels super class

0aa3f15

This can be shared between the place recognizer and the file-based sentiment detector.

Implement naive word-list sentiment scoring

2acdb51

Cache files read from disk

39ddbee

Extract top-level nlp package

1970a1e

Extract tokenizer

a8e4cd8

Fix build and compiler warnings

ae6c0c0

Use composition over inheritance

2a83de2

Add some tests for word list sentiment

d1641d7

Inline some methods

b7d2177

Generalize logger trait

00a4902

Add some more logging

f3422e6

Extract integration test spec super class

954e5e2

Handle sentiment service errors

4694d10

c-w added the in progress label Jun 7, 2017

Rename logger trait to avoid name clash with log4j

5854ec2

c-w changed the title ~~More sentiments~~ Add sentiment analysis capability for 68 more languages Jun 7, 2017

c-w requested review from kevinhartman and erikschlegel June 7, 2017 15:16

kevinhartman approved these changes Jun 8, 2017

View reviewed changes

c-w added 7 commits June 8, 2017 22:33

Pascal-case constants

cb10a83

Make function name more explicit

9ec3011

Make it explicit that return type is set

5861eab

Make tokenizer object directly usable

880c5af

Make string check more specific

d5cbd60

Extract trait for sentiment detector

c7008f8

Reduce scope of NPE check

e89573a

c-w merged commit d0d9835 into master Jun 10, 2017

c-w deleted the more-sentiments branch June 10, 2017 21:50

c-w removed the in progress label Jun 10, 2017

Add sentiment analysis capability for 68 more languages #9

Add sentiment analysis capability for 68 more languages #9

c-w commented Jun 7, 2017

kevinhartman left a comment

kevinhartman Jun 8, 2017

c-w Jun 8, 2017

kevinhartman Jun 8, 2017

c-w Jun 8, 2017

kevinhartman Jun 8, 2017

c-w Jun 8, 2017 •

edited

kevinhartman Jun 8, 2017

c-w Jun 8, 2017

kevinhartman Jun 8, 2017

c-w Jun 8, 2017 •

edited

kevinhartman Jun 9, 2017

c-w Jun 9, 2017

kevinhartman Jun 8, 2017

c-w Jun 8, 2017

kevinhartman Jun 9, 2017

c-w Jun 9, 2017

kevinhartman Jun 8, 2017

c-w Jun 8, 2017

kevinhartman Jun 8, 2017

c-w Jun 8, 2017

kevinhartman Jun 8, 2017

c-w Jun 8, 2017

kevinhartman Jun 9, 2017

c-w Jun 9, 2017

kevinhartman Jun 8, 2017

c-w Jun 8, 2017 •

edited


		logDebug(s"Analyzed text $text in language $language: $kaf")

		kaf.getEntities.toList.filter(entityIsPlace).map(_.getStr).toSet

Add sentiment analysis capability for 68 more languages #9

Add sentiment analysis capability for 68 more languages #9

Conversation

c-w commented Jun 7, 2017

kevinhartman left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

c-w Jun 8, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

c-w Jun 8, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

c-w Jun 8, 2017 • edited

Choose a reason for hiding this comment

c-w Jun 8, 2017 •

edited

c-w Jun 8, 2017 •

edited

c-w Jun 8, 2017 •

edited