WIP: Adding SRL #215

danyaljj · 2016-09-20T20:46:21Z

Pending issues to discuss:

I did not move curator-release components.
Waiting for the resources issue to be fixed. break apart illinois-common-resources #186
Moving WeightVectorUtils to SL.

Simplified the predicate candidate generation for Nom Added dummy sentence creation for Nom during SemanticRoleLabeler init Removed pre-processing during training (inside PreExtractor) Changed system output to logger Cleaned up pom.xml Removed pom from .gitignore

Resolved a clash in lexicon naming by adding the parser view (also removed the "lexicon." prefix)

Replaced console output with logger calls Updated compileModelsToJar to accommodate both Stanford and Charniak

Removed unnecessary dependencies Switched to illinois-nlp-pipeline-0.1.2 Minor fixes

Fixed model deployment

fixed a bug that prevented the TextPreProcessor from being initialized outside SRL.

-Only works with the Lagrange inference solver for now

handle exception from getParsePhrase

…SRL (otherwise exception is thrown if not present, and it usually isn't...) Added a memory test to gain some insight into whether memory is freed properly (symptoms were in pipeline); behavior seems reasonable.

@christod

updated to use updated Annotator API, which accommodates lazy initialization had to change constructor signatures, push some constructor params into ResourceManager argument updated tests to conform to new multi-sentence DummyTextAnnotation TODO: verify that nom is behaving correctly (only finds predicates in first sentence) @christod there is no rush on this -- I've deployed a snapshot for use with a snapshot of pipeline for my own purposes. See merge request !13

updated version number finalizing coordinated updates with illinois-nlp-pipeline See merge request !14

# Conflicts: # .gitignore

christos-c

I would suggest removing the curator-release components.

christos-c · 2016-09-21T18:16:22Z

srl/pom.xml

+            <groupId>edu.illinois.cs.cogcomp</groupId>
+            <artifactId>illinois-srl-models</artifactId>
+            <classifier>verb-stanford</classifier>
+            <version>5.1</version>


Make sure to change the models version (after retraining)

christos-c · 2016-09-21T18:16:53Z

srl/pom.xml

+        <dependency>
+            <groupId>edu.illinois.cs.cogcomp</groupId>
+            <artifactId>inference</artifactId>
+            <version>0.5.0</version>


Is this the latest inference package?

christos-c · 2016-09-21T18:17:31Z

srl/pom.xml

+        </dependency>
+    </dependencies>
+
+    <build>


This section is not needed since it's inherited from the parent

christos-c · 2016-09-21T18:18:33Z

srl/pom.xml

+        <dependency>
+            <groupId>edu.illinois.cs.cogcomp</groupId>
+            <artifactId>illinois-core-utilities</artifactId>
+            <version>${cogcomp-nlp-version}</version>


These cogcomp-nlp-version variables should be replaced with the current super-project version.

christos-c · 2016-09-21T18:19:24Z

srl/pom.xml

+        <dependency>
+            <groupId>edu.illinois.cs.cogcomp</groupId>
+            <artifactId>illinois-common-resources</artifactId>
+            <classifier>illinoisSRL</classifier>


Note the use of the classifier in case this needs to be addressed (see #43)

huh? I am not clean on the classifier tag usage here ...

I was using the classifier property to split the common-resources per project. Each project would use its own subunit of the common resources module.

1.3 is the one currently used in Edison and 1.4 is the one used in SRL. There is an effort to break the resources into smaller jar files. I can update it after that.

christos-c · 2016-09-21T18:25:27Z

srl/src/main/java/edu/illinois/cs/cogcomp/srl/data/LegalArguments.java

+	private final static Logger log = LoggerFactory.getLogger(LegalArguments.class);
+
+	private final Map<String, Set<String>> legalArgs = new HashMap<>();
+    private final Map<String, Set<String>> legalArgsForSense = new HashMap<>();


Indentation error: are we using the formatter plugin?

christos-c · 2016-09-21T18:26:37Z

srl/src/main/java/edu/illinois/cs/cogcomp/srl/features/ProjectedPath.java

@@ -0,0 +1,93 @@
+/**


This exists in edison.

christos-c · 2016-09-21T18:29:20Z

srl/src/main/java/edu/illinois/cs/cogcomp/srl/utilities/WeightVectorUtils.java

@@ -0,0 +1,134 @@
+/**


This should be moved to core-utilities or inference or illinois-sl.

core-utils doesn't have sl as dependency (and vice versa). Maybe we keep it here for the time being?

illinois-srl has a dependency to sl and inference (and core-utilities of course). So moving this file to one of those package would be the best solution.

I think my point is different: if you look at the file, it contains functions from both core-utils and SL. Moving this file to one of those projects require one of them to be dependency of the other.

oh I see... but the core-utilities dependencies are limited to just IOUtils and illinois-sl already has a solution for these methods (see WeightVector). So maybe just a quick fix and this file will be ready to be moved to sl

That sounds doable.

christos-c · 2016-09-21T18:30:12Z

srl/src/main/resources/features/Verb.Classifier.old.fex

@@ -0,0 +1,83 @@
+(define features


This should be removed.

christos-c · 2016-09-21T18:30:27Z

srl/src/main/resources/features/Verb.Identifier.old.fex

@@ -0,0 +1,75 @@
+(define features


This should be removed.

christos-c · 2016-10-14T06:16:30Z

srl/pom.xml

+        <dependency>
+            <groupId>edu.illinois.cs.cogcomp</groupId>
+            <artifactId>illinois-common-resources</artifactId>
+            <version>1.5</version>


Why do we need both the general illinois-common-resources, as well as the srl and the ner ones? The appropriate solution would be to pack everything we need into the srl common-resources.

Removed `SentenceDBHandler` and replaced it with the `TextAnnotationCache` interface Added a `close()` method to the interface to avoid problems with MapDB (and any future implementers of the interface)

christos-c · 2016-10-14T07:49:53Z

@danyaljj I've updated your branch from upstream and updated the cache mechanism.
NB: I have NOT tested this on my end since I don't have access to the Propbank/Treebank files; if it doesn't work just let me know and I can try to debug it on my Illinois desktop.

danyaljj · 2016-10-14T15:01:29Z

Yasss! Awesome will test it.

christos-c and others added 30 commits December 11, 2014 18:24

Initial commit

53c4ee1

Added pom.xml back to git

d31108b

Fixed an error in the WSJ train/dev sections split

7e0add5

Resolved a clash in lexicon naming by adding the parser view (also removed the "lexicon." prefix)

Minor changes

e94d688

Replaced console output with logger calls Updated compileModelsToJar to accommodate both Stanford and Charniak

Added JUnit tests

794bd04

Removed unnecessary dependencies Switched to illinois-nlp-pipeline-0.1.2 Minor fixes

Declared gurobi optional

6eeded1

Reverted back to nlp-pipeline-0.1.1

9bee145

Fixed model deployment

Fixed bug in TextPreProcessor

93352eb

fixed a bug that prevented the TextPreProcessor from being initialized outside SRL.

Fixed dash ('-') bug in Nom SRL

80fd99e

Added an option for max inference rounds

e766eb4

-Only works with the Lagrange inference solver for now

Added sense description ("name") to FrameData

09216e6

Merge remote-tracking branch 'origin/master'

0bd5517

moving to new deps 1

4a15867

moving to new deps 2

80a30f0

moving to new deps 3

b011ba6

handle exception from getParsePhrase

final moving all to new deps 4

da09783

adding column writer from old edison

39b1a5e

should compile. v5.1.3

527ac81

WIP for moving to new dep.

e9a68aa

which inference to use?

f322db1

compiles

400d6fa

config madness. Need one config? or many?

8a088e3

New config for pipeline

39993e1

this version goes upto feature extraction smoothly

dd07d30

can train sense still training identifier

8dbadf3

also try SP

3722c3e

confiugrable learner

2afb706

adding learner config

2386ec7

all models trained!

e744814

Mark Sammons and others added 12 commits August 14, 2016 02:11

updated to check for/add CLAUSE_STANFORD view before trying to apply …

662afdd

…SRL (otherwise exception is thrown if not present, and it usually isn't...) Added a memory test to gain some insight into whether memory is freed properly (symptoms were in pipeline); behavior seems reasonable.

updated icu dependency to non-snapshot

2a98e89

updated pipeline dependency

357fd39

updated version number

d23369b

Merge branch 'icuupdate' into 'master'

a008507

updated version number finalizing coordinated updates with illinois-nlp-pipeline See merge request !14

moving the files into the srl folder

1f6beab

Merge remote-tracking branch 'cogcomp/master'

2ab580c

# Conflicts: # .gitignore

adding srl to the main pom file.

458b844

update readme file

6813089

remove gitlab-ci file.

de4dab6

adding license header to the srl files.

901998f

christos-c requested changes Sep 21, 2016

View reviewed changes

khashab2 added 7 commits September 22, 2016 11:39

Merge remote-tracking branch 'cogcomp/master'

2653065

applying comments: updated pom file and removed some redundant files.

960e5c1

applying formatter on SRL.

90f920e

dropping two old fex files.

8907a86

adding a memory condition to srl tests.

f196fe4

increase memory requirement for srl tests to 8gb.

fd97201

drop curator release components.

f292cde

danyaljj mentioned this pull request Sep 29, 2016

Weight vector utils CogComp/illinois-sl#20

Merged

some clean up for pom.xml.

084d0ba

christos-c reviewed Oct 14, 2016

View reviewed changes

christos-c added 2 commits October 14, 2016 08:11

Merge remote-tracking branch 'upstream/master' into adding-srl

7c40a69

Changed SRL's TA cache mechanism

24ba53e

Removed `SentenceDBHandler` and replaced it with the `TextAnnotationCache` interface Added a `close()` method to the interface to avoid problems with MapDB (and any future implementers of the interface)

christos-c assigned danyaljj Oct 14, 2016

do not initialize multiple caches.

8252cb9

danyaljj mentioned this pull request Apr 5, 2017

Fix evalution issue with PredicateArgumentEvaluator. #413

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

WIP: Adding SRL #215

WIP: Adding SRL #215

danyaljj commented Sep 20, 2016 •

edited

christos-c left a comment

christos-c Sep 21, 2016

christos-c Sep 21, 2016

christos-c Sep 21, 2016

christos-c Sep 21, 2016

christos-c Sep 21, 2016

danyaljj Sep 22, 2016

christos-c Sep 22, 2016

danyaljj Sep 29, 2016 •

edited

christos-c Sep 21, 2016

danyaljj Sep 22, 2016

christos-c Sep 21, 2016

christos-c Sep 21, 2016

danyaljj Sep 22, 2016

christos-c Sep 22, 2016

danyaljj Sep 22, 2016

christos-c Sep 23, 2016

danyaljj Sep 23, 2016

christos-c Sep 21, 2016

danyaljj Sep 22, 2016

christos-c Sep 21, 2016

danyaljj Sep 22, 2016

christos-c Oct 14, 2016

christos-c commented Oct 14, 2016

danyaljj commented Oct 14, 2016

WIP: Adding SRL #215

Are you sure you want to change the base?

WIP: Adding SRL #215

Conversation

danyaljj commented Sep 20, 2016 • edited

christos-c 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

danyaljj Sep 29, 2016 • 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

christos-c commented Oct 14, 2016

danyaljj commented Oct 14, 2016

danyaljj commented Sep 20, 2016 •

edited

danyaljj Sep 29, 2016 •

edited