search

CogComp / saul

Saul : Declarative Learning-Based Programming
Other
64 stars 18 forks source link

Updating the version of CCG dependencies #363

Closed danyaljj closed 8 years ago

danyaljj commented 8 years ago

Incremented the illinois-cogcomp-nlp version to the latest and had to fix some of the tests. Some of the tests I couldn't fix (Some in SRL). @kordjamshidi Need your help. Could you have a look into the failed tests? You can checkout the code from my fork. Here are the tests that fail:

kordjamshidi commented 8 years ago

ok, sure. I am going to checkout. @danyaljj

kordjamshidi commented 8 years ago

For some reason the ParsePath feature in Edison does not return the correct value.

kordjamshidi commented 8 years ago

Do you have tests in Edison itself that shows this feature works correctly there?

kordjamshidi commented 8 years ago

I trace it inside Edison. It seems the problem is again related to the source and target of the relation. For some reason when the relation is passed to this feature, and it starts from the source argument i.e. predicate and extracts the target argument then it uses the parse tree to again get the source and it misses the source at this point. It finds the path between the argument and itself. So I guess this should be a bug in Edison.

danyaljj commented 8 years ago

Found the error for parse feature extractor. Will fix it. What do you think about other test failures in SemanticRoleLabeling.ModelsTest?

kordjamshidi commented 8 years ago

Seems this is also outside Saul, related to the annotator initialization(?!).

An exception or error caused a run to abort: edu.illinois.cs.cogcomp.annotation.Annotator.initialize(Ledu/illinois/cs/cogcomp/core/utilities/configuration/ResourceManager;)V java.lang.AbstractMethodError: edu.illinois.cs.cogcomp.annotation.Annotator.initialize(Ledu/illinois/cs/cogcomp/core/utilities/configuration/ResourceManager;)V

danyaljj commented 8 years ago

Found it; it is related to the changes made here. Will be fixed after this pr.

kordjamshidi commented 8 years ago

@danyaljj: do you think this issue is solved now?

kordjamshidi commented 8 years ago

Actually, I just added the excludes, for me the test still fails as mentioned in issue #365 .

bhargav commented 8 years ago

Changes look good. But the test failed due to unable to load POS Baseline models. I believe since the class POSBaselineLearner changed namespace, we might have to train and upload new models.

danyaljj commented 8 years ago

@bhargav fixed the error with POS models. Now it's failing for some weird error in ConstraintsTest. 

[info] Exception encountered when attempting to run a suite with class name: edu.illinois.cs.cogcomp.saulexamples.nlp.SemanticRoleLabeling.ConstraintsTest *** ABORTED ***
[info]   java.lang.NoSuchMethodError: edu.illinois.cs.cogcomp.nlp.corpusreaders.TextAnnotationReader.<init>(Ljava/lang/String;)V
[info]   at edu.illinois.cs.cogcomp.nlp.corpusreaders.PennTreebankReader.<init>(PennTreebankReader.java:79)
[info]   at edu.illinois.cs.cogcomp.nlp.corpusreaders.PennTreebankReader.<init>(PennTreebankReader.java:69)
[info]   at edu.illinois.cs.cogcomp.nlp.corpusreaders.AbstractSRLAnnotationReader.<init>(AbstractSRLAnnotationReader.java:86)
[info]   at edu.illinois.cs.cogcomp.nlp.corpusreaders.PropbankReader.<init>(PropbankReader.java:108)
[info]   at edu.illinois.cs.cogcomp.saulexamples.data.SRLDataReader.<init>(SRLDataReader.java:36)
[info]   at edu.illinois.cs.cogcomp.saulexamples.nlp.SemanticRoleLabeling.PopulateSRLDataModel$.apply(PopulateSRLDataModel.scala:138)
[info]   at edu.illinois.cs.cogcomp.saulexamples.nlp.SemanticRoleLabeling.SRLApps$.<init>(SRLApps.scala:69)
[info]   at edu.illinois.cs.cogcomp.saulexamples.nlp.SemanticRoleLabeling.SRLApps$.<clinit>(SRLApps.scala)
[info]   at edu.illinois.cs.cogcomp.saulexamples.nlp.SemanticRoleLabeling.SRLClassifiers$argumentTypeLearner$.<init>(SRLClassifiers.scala:33)
[info]   at edu.illinois.cs.cogcomp.saulexamples.nlp.SemanticRoleLabeling.SRLClassifiers$argumentTypeLearner$.<clinit>(SRLClassifiers.scala)
[info]   ...

Any ideas?

kordjamshidi commented 8 years ago

It seems the errors initiated from corpusreaders, and should be with the package renaming, relocating I guess..

danyaljj commented 8 years ago

Closing to open another PR.