bugs with multiple NERAnnotators per process space

[cowchipkid]

There were some bugs with NER model loading, and all annotators were sharing the same NER model.

[nitishgupta]

Build is failing, btw. @cowchipkid

[cowchipkid]

Ya, I am seeing that as well on my machine. Something broken on merge. I’ll update soon

On Aug 2, 2018, at 2:30 PM, Nitish Gupta notifications@github.com wrote:

Build is failing, btw. @cowchipkid https://github.com/cowchipkid — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/CogComp/cogcomp-nlp/pull/675#issuecomment-410041550, or mute the thread https://github.com/notifications/unsubscribe-auth/ACdHS1JsRayJOZW9UU7fX3gaI55RkfMtks5uM1NmgaJpZM4Vs3IW.

[nitishgupta]

@cowchipkid: looks like there was some orthogonal update to NER which now causes two (simple) merge conflicts. Could you please have a look?

@mssammon: Can we merge this after Tom fixes the merge request?

[mssammon]

@nitishgupta yes, just waiting for conflict resolution.

[cowchipkid]

@danyaljj This is ready to merge, can you put your OK on it and merge?

[cowchipkid]

@danyaljj @mssammon I've updated to version 4.0.12, fixed runNER so when tests complete, we are good.

[mssammon]

Can merge when CI build finishes/shows green.

[nitishgupta]

@cowchipkid I'm assuming you've tested multiple Annotators with different models running simultaneously. I have a question regarding the shared config you mentioned yesterday. What does that entail? How do we change the config programmatically when initializing the Annotators? What are the configs we can change, and what are their possible values? Sorry, if this information is listed somewhere, but a quick read through the README didn't give me much info.

[cowchipkid]

@nitishgupta Yes, different annotators will not use different models stored as fields of the NERAnnotator as they did before. However, other configuration properties like the one that enables the use of the level 2 model, the one that breaks sentences on newlines and so on are all shared. And there is nothing you can do about it. Wish I had a better answer for you, if it were easy, I would just make the config class factory provided keyed on view name. But that's not a one day job, as some of the low level feature generators use these configuration properties and feature generators do not have access to the view name.

[danyaljj]

Looks good to me. Merge when you feel happy about it.

[nitishgupta]

@cowchipkid Thanks for your response.! I think I understand your point. Is there still a way to find out what the modifiable config parameters are, their possible values, and how to change them programmatically?

[cowchipkid]

@nitishgupta All these properties are defined in a ParametersForLbjCode, all these properties are static fields of this class, and a static embedded properties class. From where I sit, coming up with a good solution that works consistently is the most expedient way to fix this, it is impossible for multiple annotators to share static variables without tons of monitor locks, defeating the usefulness of multi-threading. I believe a much better solution is to completely re-architect this properties container, rather than a singleton, build a factory interface to serve up instances per annotator. The only quandary is then how the low level feature generators can share a key with the annotator to fetch the instance of the properties from the factory. If I had more time, I could fix this, but alas my last day...

[nitishgupta]

Okay, got it. Thank you.