search

grammarly / gector

Official implementation of the papers "GECToR – Grammatical Error Correction: Tag, Not Rewrite" (BEA-20) and "Text Simplification by Tagging" (BEA-21)
Apache License 2.0
891 stars 216 forks source link

Update allennlp to newer version #163

Open marekrydlewski opened 2 years ago

marekrydlewski commented 2 years ago

First, thanks for your amazing work.

Do you plan to update allennlp dependency to a newer version? (e.g. 2.x.x)? This project uses an archaic version 0.8.4 which causes quite a few security problems & it is problematic to work with Python newer than 3.7.

I've tried to make the update on my own, but there are several changes in allennlp API as well as additional tweaks of allennlp classes in this repository makes it hard for someone not accustomed to the codebase.

The old version of allennlp stops us from using GECToR in our company, thank you!

skurzhanskyi commented 2 years ago

Hi @marekrydlewski Thanks for the feedback. Indeed, transition to the newest AllenNLP would require significant code rewrites. Unfortunately, we do not plan to update it. From the very beginning, this repository was treated as a "research paper repository". At the same time, if you have the opportunity to update the code (possibly collaborating with others), we would be happy to review your changes. This was previously done to update the transformer version. Thanks

marekrydlewski commented 2 years ago

Thanks, @skurzhanskyi There is a chance we will try to update the code, await pull requests then

skurzhanskyi commented 2 years ago

Nice to hear it 🔥

damien2012eng commented 2 years ago

Hi @marekrydlewski
Im wondering how far you've done for updating the code. Our team is also interested to do the same thing.

Jason3900 commented 1 year ago

@marekrydlewski @damien2012eng Hey, I have a re-implementation of the GECToR code, which drops all AllenNLP dependencies and use vanilla pytorch to construct the code. For acceleration and distributed training, I use deepspeed to simplify the whole training process. I test all the codes with my experiments, it turns out to be much faster and achieves compatible performance compared to this repo. If you're interested, you can check my repo here: https://github.com/Jason3900/FastGECToR . @skurzhanskyi And if it's useful, I can also make a merge request to your repo. BTW, the AllenNLP is in maintenance mode.

damien2012eng commented 1 year ago

@Jason3900 This is really cool. Thanks for sharing!

skurzhanskyi commented 1 year ago

@Jason3900, great news! You did a big piece of work, and I see that code differs significantly. For reproducibility, I suggest that we treat your work as a separate repository and add the link to it in our README. WDYT?

Jason3900 commented 1 year ago

@skurzhanskyi That would be great! Thanks!

Jason3900 commented 1 year ago

@damien2012eng You are welcome. Glad to be of help!

skurzhanskyi commented 1 year ago

@Jason3900 Thank you for your work!

skurzhanskyi commented 1 year ago
image
Jason3900 commented 1 year ago

@skurzhanskyi You are welcome! Thanks for the excellent work on GEC!

Jason3900 commented 1 year ago

@skurzhanskyi Hey! Sorry to bother you again. My team would like to public my project on https://github.com/cofe-ai/fast-gector . And I'll maintain it from now on. Therefore, is it possible to update your readme to change the link?

skurzhanskyi commented 1 year ago

Sure, will do