Discard Labels, add Resource Factory #725
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
are not interested in from a model. Added a unit test to validate.
…d NER overall by 2 - 3%.
NEWord object via the ColumnFileReader, but with no changes to the features.
…e shared more easily, tests included. Also some minor performance (wallclock time) optimizations.
…xes to the NER resource manager stuff.
|
Hi Tom, there are a few issues I found during mvn install:
Thanks, Ben |
|
There is an old PR for that from back in July. It never got merged. Semaphore seems to be failing it, but I have no idea why, and I don’t see how to get the failure details out of Semaphore. It does of course pass unit tests on my computer.
Can anybody tell me why there is a Semaphore failure?
… On Sep 29, 2019, at 11:15 AM, Xuanyu Zhou ***@***.***> wrote:
Hi Tom, there are a few issues I found during mvn install:
LBJava 1.3.2 does not exist in the current cogcomp m2repo, there is an 1.3.2-SNAPSHOT instead. Thus your current change will lead to jar not found.
NER compilation failed on my end (after I rolled LBJava back to 1.3.1) with error NERAnnotator.java:[132,41] cannot find symbol [ERROR] symbol: method pruneUnusedLabels(java.util.ArrayList<java.lang.String>) [ERROR] location: variable taggerLevel1 of type edu.illinois.cs.cogcomp.lbjava.learn.SparseNetworkLearner. I am assuming it's related to the LBJava generated files. It may work for you because you already have the correct versions of those files, and could you try after deleting them? (as the repo does not have them).
Thanks, Ben
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub <#725?email_source=notifications&email_token=AATUOS6XGE7QNTIA673NWKTQMDICZA5CNFSM4IYML4C2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD73YY5I#issuecomment-536317045>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AATUOS7FJQ7NIT4E5EAXFILQMDICZANCNFSM4IYML4CQ>.
|
|
Which PR are you referring to? Given the current situation, do you have any suggestions on how we should move forward? |
|
Reference via CogComp/lbjava#110 <CogComp/lbjava#110>, I have also added you as a reviewer, so you should get an email on that shortly. Daniel was trying to figure out why Semaphore was failing last I heard. And Daniel spoke Semaphore fluently, so I’m not quite sure where to go from here.
… On Sep 29, 2019, at 3:42 PM, Xuanyu Zhou ***@***.***> wrote:
Which PR are you referring to? Given the current situation, do you have any suggestions on how we should move forward?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub <#725?email_source=notifications&email_token=AATUOS6KPKHGLFLTLZEELG3QMEHKZA5CNFSM4IYML4C2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD736G6A#issuecomment-536339320>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AATUOS75YEYTB5TBQ4A2MLLQMEHKZANCNFSM4IYML4CQ>.
|
…g the corrective synchronizations would counteract any already small performance benefits from the caching, I simply removed it.
|
@HeglerTissot can we get this thing merged? Am I missing some procedural change or something? It should be a big lift to get things merged, and I see 3 outstanding PRs for lbjava as well. I think semaphore is having trouble with the LBJava PR for reasons unknown, Daniel was looking at that, but not any more... |
added 3 commits
November 12, 2019 17:28
…ere was a long standing bug in ExceptionlessInputStream that would leave a compressed zip file open after loading the model. The ZipFile object should be close explicitly, and was not.
|
I'm now reviewing this pull request. And I get this error: Could you please fix this first? @cowchipkid |
|
This is the result of another PR from July (CogComp/lbjava#110) of last year never getting merged because of a Semaphore issue. My personal feeling is that the LBJava branch should be merged first, but I have been trying to get all this stuff merged literally for months and months and months, so in the interest of expedience, you could check out branch for the LBJava code, and do a mvn install on that?
… On Feb 19, 2020, at 9:17 PM, Xiaodong Yu ***@***.***> wrote:
I'm now reviewing this pull request. And I get this error:
[ERROR] Failed to execute goal on project LBJava-NLP-tools: Could not resolve dependencies for project edu.illinois.cs.cogcomp:LBJava-NLP-tools:jar:4.0.19: Could not find artifact edu.illinois.cs.cogcomp:LBJava:jar:1.3.3 in CogcompSoftware (http://macniece.seas.upenn.edu/m2repo/ <http://macniece.seas.upenn.edu/m2repo/>)
Could you please fix this first?
Thanks,
Xiaodong
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub <#725?email_source=notifications&email_token=AATUOS34NRZGPFILLBO3YDDRDXY4FA5CNFSM4IYML4C2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEMKSNBQ#issuecomment-588588678>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AATUOS7ORHORN7ZN4PE7LUTRDXY4FANCNFSM4IYML4CQ>.
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Labels of no interest can now be discarded using a property, there is a resource factory that more effectively organizes shared resources, and some wall clock time performance improvements.