emanjavacas / pie

A fully-fledge PyTorch package for Morphological Analysis, tailored to morphologically rich and historical languages.
MIT License
22 stars 10 forks source link

Update torch/1.6.0 #75

Closed PonteIneptique closed 3 years ago

PonteIneptique commented 3 years ago

Hey @emanjavacas I know you are probably quite busy with the PhD but if you have time, this PR allows for using later version of torch. I am stupidly stuck with models from 1.6.0 from my other branches which I can't use if I install pie properly through pip :/

Hoping all is alright on your side :)

I prebumped the version

PonteIneptique commented 3 years ago

Hey Enrique, Quick, very quick bump, if you can merge and release on pypi. Torch 1.3.1 and previous were never released on Py3.8 and further, which are the only version shipped on 20.04. It's not a specific issue of Pie but that would probably make the life of some users easier (specifically users who do not have a huge knowledge of Python / Ubuntu)

Not an issue specific to Torch though : https://stackoverflow.com/questions/61430166/python-3-7-on-ubuntu-20-04

PonteIneptique commented 3 years ago

Well, the thing is, the torch.jit.load seems to be a little bit broader than 1.6 vs 1.3 or even 1.1. There seems to be some model which, when saved, create issues at loading time. I am unable to tell with 100% certainty that the bug for loading was caused by 1.6.0 vs 1.3.1. All I can assert for sure is that torch.jit.load helped avoid the issue. On top of that, it seems that torch.jit.load has been around for quite some time: at least 1.0.0. So I really don't understand the source of the problem, I just applied a patch that was recommended somewhere a month ago (which, of course, I can't find again now.)

PonteIneptique commented 3 years ago

Actually, looking back, it seems the jit.load is definitely not specific to the issue of bumping. But it allows at least for loading JIT models, which is not breaking things :)

emanjavacas commented 3 years ago

As far as I know there is no use of jit in pie. So, what's the use case for jit-loading?

PonteIneptique commented 3 years ago

I've cleaned the history, I think it's right that it's not the place where we should add jit support or not, so here you go, a soft update ;)

PonteIneptique commented 3 years ago

If I may bump :)

gabays commented 3 years ago

I agree with @PonteIneptique . It would be useful. Thx

emanjavacas commented 3 years ago

Hey, sorry guys, I hadn't seen this one. Last PhD weeks here.

PonteIneptique commented 3 years ago

Here you go, feel free to test and squash (The logs are too weird for it not to be squashed)

emanjavacas commented 3 years ago

Sorry last thing, have you run the test to check that 1.7 doesn't break anything?

PonteIneptique commented 3 years ago

I have ran multiple time trainings with torch 1.7.0 but let me relaunch tests one last time

PonteIneptique commented 3 years ago

It's good, tests run as well. I added the patch of 1.7.1 in authorized Torches.

PonteIneptique commented 3 years ago

And good luck with the PhD !

emanjavacas commented 3 years ago

Thanks! It's pushed to pypi already as well!

PonteIneptique commented 3 years ago

PonteIneptique commented 3 years ago

I think you need to bump before release. 3.7.0 was already taken in september ;)

emanjavacas commented 3 years ago

mmh, I see 0.3.6 since september, and 0.3.7 since last week.

PonteIneptique commented 3 years ago

If you look at https://github.com/emanjavacas/pie/compare/v0.3.7...master it shows that the upgrade of torch was post-0.3.7

And when I try to install with Py3.8 :

    The conflict is caused by:
    nlp-pie 0.3.7 depends on torch<1.4.0 and >=1.3.1
emanjavacas commented 3 years ago

It seems i forgot to pull before running twine. Anyway, it should be done now

On Tue, Feb 16, 2021 at 8:58 AM Thibault Clérice notifications@github.com wrote:

If you look at v0.3.7...master https://github.com/emanjavacas/pie/compare/v0.3.7...master it shows that the upgrade of torch was post-0.3.7

And when I try to install with Py3.8 :

The conflict is caused by:
nlp-pie 0.3.7 depends on torch<1.4.0 and >=1.3.1

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/emanjavacas/pie/pull/75#issuecomment-779656797, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABPIPIYU5FX7OPQ2Q5OCNPTS7IQQ7ANCNFSM4SFX2MBQ .

-- Enrique Manjavacas

PonteIneptique commented 3 years ago

It works ! Thanks :) Good luck with the PhD, come back to us when you have finished so we can merge PR ;)