search

CogComp / lbjava

Learning Based Java (LBJava)
http://cogcomp.cs.illinois.edu/page/software_view/LBJava
Other
13 stars 17 forks source link

Minor change to `equals` function in Feature #50

Closed danyaljj closed 8 years ago

danyaljj commented 8 years ago

We had some issues in Saul when loading the model: https://github.com/IllinoisCogComp/saul/issues/235

After serialization for each instances, we do feature extraction. Then in order to do prediction we need to lookup the features in the Lexicon, but our classifier never finds the features in the Lexicon; the culprit is this like in the equals function of Feature: https://github.com/IllinoisCogComp/lbjava/blob/master/lbjava/src/main/java/edu/illinois/cs/cogcomp/lbjava/classify/Feature.java#L378

If you change this line to f.generatingClassifier.equals(generatingClassifier) things would start to work. (note the asserts right before this line). In other words, there are two generatingClassifier set in the code which have the same string, but different addresses.

One of the generatingClassifier is internal for Lexicon and it is set when saving the lexicon on disk and reading it back.

LBjava compares addresses of generatingClassifier to make sure everything (Lexicon and feature generators) are compatible. But in Saul, the feature extractors are already defined in DataModel, while Lexicon is being loaded from file. So this check creates problems for Saul when loading models. On the other hand relaxing this check makes LBJava less safe, although I think this usually shouldn't make any problems.

danyaljj commented 8 years ago

Heard from @christos-c . Will merge when Semaphore is happy.