Skip to content
This repository has been archived by the owner on Dec 16, 2022. It is now read-only.

Adding the wikitables parser #1114

Merged
merged 2 commits into from
Apr 21, 2018

Conversation

matt-gardner
Copy link
Contributor

Performance of the parser still doesn't match the original model, but it should just be minor tweaks from here. It'll be a lot easier to manage git if this is merged into master now.

I didn't really change anything in the code, so this is just bringing things over from the wikitables branch, and it has all already been reviewed.

There are a few minor changes to trainer.py that I don't think got a close enough look previously, and so @DeNeutoy, if you want to give the changes I made there a look, that'd be great.

Copy link
Contributor

@DeNeutoy DeNeutoy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM


loss = self._batch_loss(batch, for_training=False)
val_loss += loss.data.cpu().numpy()
if loss is not None:
# You shouldn't necessarily have to compute a loss for validation, so we allow for
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually when is this used?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is used by the wikitables parser (which is why it's part of this PR). Our training loss is maximum marginal likelihood, but we don't have a set of candidate logical forms for all of the validation instances. We still can calculate accuracy, though, and we want to include all of the instances in the validation set, to get accurate metrics. So we need the training code to not crash when we're evaluating instances at validation time for which we can't actually compute a loss.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

makes sense 👍

@matt-gardner matt-gardner merged commit 0df8978 into allenai:master Apr 21, 2018
@matt-gardner matt-gardner deleted the add_wikitables_parser branch April 21, 2018 15:47
gabrielStanovsky pushed a commit to gabrielStanovsky/allennlp that referenced this pull request Sep 7, 2018
* Adding the wikitables parser

* Fix dockerfile and docs
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants