search

CoderPat / structured-neural-summarization

A repository with the code for the paper with the same title
MIT License
74 stars 26 forks source link

Fix issues pointer by github users #20

Closed CoderPat closed 5 years ago

CoderPat commented 5 years ago

Fixes #2, #18, #16, #19 , #2

CoderPat commented 5 years ago

Will leave this open until I have time to test this or some other user confirms this solves the issues

ioana-blue commented 5 years ago

My tensorflow version complained when I used None down the line. I had to use an empty dictionary.

CoderPat commented 5 years ago

You mind providing the error message? It shouldn't happen...

CoderPat commented 5 years ago

And thank you for all the feedback provided while using the code

CoderPat commented 5 years ago

@ioana-blue good news, found the issue that was making your inference disaligned, should be fixed now

ioana-blue commented 5 years ago

@CoderPat excellent news! I'm going to try to get to this code this week.

Can you please explain a bit the fix? I saw the removal of the params in infer, but I'm not sufficiently familiar with the code to understand the implications. I'd like to understand the code a bit more. TIA!

CoderPat commented 5 years ago

I was setting the shuffle buffer, which was causing the input tot be shuffled (this is used for training)

ioana-blue commented 5 years ago

oh, ok, that makes sense. are you ICLR results affected by this? I was using them as guidelines on where the performance I get should be.

CoderPat commented 5 years ago

No. The ICLR results were done mostly with OpenGNN alone. However while running the experiments, I found that the interface was a bit cumbersome for experimentation and so this repo was introduced to make life easier for people trying to replicate the paper (hence all this uncatched bugs)

CoderPat commented 5 years ago

If u don't mind I'll close this PR, and open a new one for the checkpoint problem

ioana-blue commented 5 years ago

Sounds good.