Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update allennlp to newer version #163

Open
marekrydlewski opened this issue Jul 18, 2022 · 14 comments
Open

Update allennlp to newer version #163

marekrydlewski opened this issue Jul 18, 2022 · 14 comments

Comments

@marekrydlewski
Copy link

marekrydlewski commented Jul 18, 2022

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
Copy link
Collaborator

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
Copy link
Author

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

@skurzhanskyi
Copy link
Collaborator

Nice to hear it 🔥

@damien2012eng
Copy link

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

@Jason3900
Copy link

Jason3900 commented Oct 2, 2022

@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
Copy link

@Jason3900 This is really cool. Thanks for sharing!

@skurzhanskyi
Copy link
Collaborator

@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
Copy link

@skurzhanskyi That would be great! Thanks!

@Jason3900
Copy link

Jason3900 commented Oct 8, 2022

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

@skurzhanskyi
Copy link
Collaborator

skurzhanskyi commented Oct 8, 2022

@Jason3900 Thank you for your work!

@skurzhanskyi
Copy link
Collaborator

image

@Jason3900
Copy link

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

@Jason3900
Copy link

@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
Copy link
Collaborator

Sure, will do

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants