1st order cky implementation

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

1st order cky implementation #83

Closed teffland closed 3 years ago

teffland commented 3 years ago

Hi,

I'd like to contribute this implementation of a first-order cky-style crf with anchored rule potentials: $\phi[i,j,k,A,B,C] := \phi(A{i,j} \rightarrow B{i,k}, C{k+1,j})$.

I also added code to the _Struct class that allows calculating marginals even if input tensors don't require a gradient (i.e., after model.eval())

Please let me know if you'd like to see any changes.

Thanks, Tom

teffland commented 3 years ago

Great, excited to contribute!

Re: your questions

Reshape: I can definitely try this with view. Just curious, why does reshape worry you? I thought reshape was just a more robust version of view that creates a contiguous copy in cases where view would fail.
Repeat: true, I can try without and it should work I think, I guess I just naturally tend towards implementations that go through all of the steps explicitly but I should get better about trusting broadcasting.
One sum: this is definitely doable, will make that change
I did write some tests locally in a notebook a while back. I can dig those up and incorporate them. By enumerate and score, you mean check that the dp partition matches the partition computed by explicitly enumerating all structures, correct?

Thanks for the feedback, Tom

srush commented 3 years ago

I can help with testing. Yes, all the algorithms in torch struct test that the enumeration has the same marginals. Its helped catch a ton of bugs.

Reshape- maybe they changed this, earlier version caused problems.

teffland commented 3 years ago

Cool, help with the testing would nice but I can also take a crack at it later. When I get to it I'll also incorporate the suggested changes, (and will try it out with view).

srush commented 3 years ago

Hey sorry, I flaked on helping out here. Send me an email if you want to work on it more. I'm going to do some repo maintenance this week.

teffland commented 3 years ago

Hey, no worries same here. Will send an email

teffland commented 3 years ago

included in https://github.com/harvardnlp/pytorch-struct/pull/93