kmkurn / pytorch-crf

(Linear-chain) Conditional random field in PyTorch.
https://pytorch-crf.readthedocs.io
MIT License
935 stars 151 forks source link

Cast mask tensor to bool in torch.where #95

Closed erickrf closed 1 year ago

erickrf commented 2 years ago

This fixes #92

kmkurn commented 2 years ago

Thanks for the PR! Looks good but I have to confirm it works for earlier PyTorch versions. This is usually done by Travis CI but since the move to travis.com the builds seem to have stopped. It'll be while before I can fix it ☹️

kmkurn commented 2 years ago

Hi, I've setup Github CI to replace Travis, so do you mind rebasing this PR onto the master branch?

erickrf commented 2 years ago

Done!

erickrf commented 2 years ago

Oh... I hadn't realized tensor.bool() wasn't available in previous versions of pytorch. By the way, the project requirements don't say anything about pytorch version, this is only set in the test config. Do you want to support pytorch 1.0?

kmkurn commented 2 years ago

project requirements don't say anything about pytorch version, this is only set in the test config

Yes, this is intentional as there are several ways to install pytorch depending on the platform and if GPU support is needed. Can't really put all these in a single requirement (not sure if this has changed?).

Do you want to support pytorch 1.0?

As pytorch is now at v1.10, perhaps dropping support for much earlier versions like v1.0 isn't a bad idea. Do you know when Tensor.bool() was introduced?

myedibleenso commented 2 years ago

Do you know when Tensor.bool() was introduced?

@kmkurn , based off of https://github.com/pytorch/pytorch/pull/16810 and https://github.com/pytorch/pytorch/commit/aa6403bae6aeb452bd464a6ec4913d1b69548aca, it looks like it was introduced in v1.1.0

kmkurn commented 2 years ago

Thanks @myedibleenso. The builds for PyTorch v1.1 have failed. I can't see the log any more so I'm not sure why. I can't seem to re-run the builds either ☹️