coli-saar / am-parser

Modular implementation of an AM dependency parser in AllenNLP.
Apache License 2.0
30 stars 10 forks source link

CreateCorpus obsolete? #68

Open alexanderkoller opened 5 years ago

alexanderkoller commented 5 years ago

Every CreateCorpus class we have also seems to have a sister class CreateCorpusParallel. Do we ever use the CreateCorpus classes? Could we simply delete them all? I worry that they are falling out of sync.

namednil commented 5 years ago

That's a fair point. However, we need a non-parallel version for debugging of heuristics, otherwise the output is not interpretable because it blends messages from different graphs.

alexanderkoller commented 5 years ago

At least after the MRP deadline, let's get rid of the non-parallel versions. We can get sequential output if we fix the number of CPU cores to 1. Also, if we write error reports to files for the individual instance IDs, we can also work around the "blending" issue.

I see a really high risk of unexplainable bugs if we keep different versions of CreateCorpus around that duplicate so much code between each other.