CogComp / cogcomp-nlp

CogComp's Natural Language Processing Libraries and Demos: Modules include lemmatizer, ner, pos, prep-srl, quantifier, question type, relation-extraction, similarity, temporal normalizer, tokenizer, transliteration, verb-sense, and more.
http://nlp.cogcomp.org/
Other
471 stars 142 forks source link

ServerClientAnnotator causes side-effects #500

Open bhargav opened 7 years ago

bhargav commented 7 years ago

Line 156 causes some un-intended side-effects. Since the ServerClientAnnotator creates a temporary TextAnnotation (from the server response), the new view that is added still refers to newTA.

Consider the following scenario:

TextAnnotation ta1  = ... // With views ViewNames.POS, ViewNames.TOKENS, ViewNames.SENTENCE only

ServerClientAnnotator annotator = new ServerClientAnnotator()
annotator.setViews(ViewNames.NER_CONLL)

annotator.addView(ta1) // This creates an instance of newTA on line 151.

ta.getView(ViewNames.POS).getTextAnnotation != ta.getView(ViewNames.NER_CONLL).getTextAnnotation

ta.getView(ViewNames.POS).getTextAnnotation holds a reference to ta1, while ta.getView(ViewNames.NER_CONLL).getTextAnnotation is a reference to newTA. This causes discrepancies while using the generated view's getTextAnnotation method downstream.

danyaljj commented 7 years ago

Truuu. Could you clarify more on "discrepancies"? (Like give example)

bhargav commented 7 years ago

In Saul, one issue I had was that I was adding text annotations as keys to a HashMap for some evaluation and when I try to access the HashMap using the ta.getView().getTextAnnotation from views added using ServerClientAnnotation, the HashMap says key not found. The newTA created intermediate has empty getId, getCorpusId and empty set of attributes. So hashcode and equals are different than the original document. Not sure if this affects lots of people.

danyaljj commented 7 years ago

Ah I see. What's your suggestion on this?

danyaljj commented 7 years ago

How about using annotate function inside ServerClientAnnotator? Does it have this issue too?