harvardnlp / pytorch-struct

Fast, general, and tested differentiable structured prediction in PyTorch
http://harvardnlp.github.io/pytorch-struct
MIT License
1.11k stars 93 forks source link

Alignment CRF error #106

Open jdegange opened 3 years ago

jdegange commented 3 years ago

Looks like there is an error now on install for AlignmentCRF.

https://github.com/harvardnlp/pytorch-struct/blob/master/notebooks/CTC_with_padding.ipynb

Find marginals (see uncertainty from randomness)
show(dist.marginals, 1)

Error: ` ~/opt/anaconda3/lib/python3.7/site-packages/torch_struct/alignment.py in _dp_scan(self, log_potentials, lengths, forcegrad) 98 point = (l + M) // 2 99 --> 100 charta[1][:, b, point:, 1, ind, :, :, Mid] = semiring.one( 101 charta[1][:, b, point:, 1, ind, :, :, Mid] 102 )

AttributeError: type object 'LogSemiring' has no attribute 'one_' `

JohnReid commented 3 years ago

Turns out the one_ method was removed (in #105 I think). You can change the calling code on line 100 in torch_struct/alignment.py to what the method used to do:

-            charta[1][:, b, point:, 1, ind, :, :, Mid] = semiring.one_(
-                charta[1][:, b, point:, 1, ind, :, :, Mid]
-            )
+            # semiring.one_() was removed
+            charta[1][:, b, point:, 1, ind, :, :, Mid] = charta[1][:, b, point:, 1, ind, :, :, Mid].fill_(0)

However despite this progress I still have problems when calculating the marginals using genbmm as my tensors are not cuda.

JohnReid commented 3 years ago

@srush would it be worth adding a test for AlignmentCRF or Alignment that would have failed for #105? I'm happy to send a PR.

srush commented 3 years ago

Oh, Unfortunately alignment crf only runs on gpu, so the tests don't catch this.

Are you using it? Wondering if I should just remove it until I figure out a better approach.

JohnReid commented 3 years ago

I was hoping to use it. Can the tests not run on the GPU or am I missing the point?

srush commented 3 years ago

Yes, they should run on GPU, but they don't run automatically. If you want to fix them and check that they pass that would be great. Otherwise, I'll take a look (but likely not until next week).

JohnReid commented 3 years ago

See #109. I'm not 100% sure this is what you suggested I do but it is a starting point at least.