kmkurn / pytorch-crf

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

Transition score for masked timesteps #53

Closed Anshul-Gupta24 closed 2 years ago

Anshul-Gupta24 commented 4 years ago

Hi, thanks for the great library. I was going through your code and had a doubt regarding the computation of score when a timestep is masked. Specifically, in line 192 of torchcrf/__init_\.py:

score += self.transitions[tags[i - 1], tags[i]] * mask[i]

Shouldn't this be:

score += self.transitions[tags[i - 1], tags[i]] * mask[i] * mask[i-1]

As we do not want transition score to be calculated when the previous timestep was masked. Thanks!

kmkurn commented 4 years ago

Hi, thanks for checking out the library! The mask is assumed to be a length mask, so it should be ones followed by (possibly none) zeroes. So, if mask[i-1]==0 then mask[i]==0 as well. In this case, the two equations are equivalent.

Anshul-Gupta24 commented 4 years ago

I see, thanks for the clarification. I currently have some masked values in between timesteps so this might affect outputs there. Could the mask be converted to a general purpose one by making the change above (although the first timestep cannot still be masked). Let me know what you think.

kmkurn commented 4 years ago

I guess you could, but then the transition code might get complicated since you can have arbitrary number of masked time steps between two valid (unmasked) ones. Off the top of my head, I'm not sure if there's a way to do so without changing the code drastically.