[TGA] Abstract TGA candidate ranking and fix ranking for BART #3455
Conversation
parlai/agents/bart/bart.py
Outdated
| cands, _ = self._pad_tensor(batch.candidate_vecs[i]) | ||
| scores, _ = self.model.decode_forced(enc, cands) | ||
| # ignore the score for the start token | ||
| scores = scores[:, 1:, :] |
There was a problem hiding this comment.
is this the only line that is different?
I'm wondering now if we can solve all of these issues by just overriding BartModel.output to do this computation...
that would allow us to get rid of the duplicate compute_loss and _construct_token_losses functions as well
There was a problem hiding this comment.
the other change is that it needs reshaping...
There was a problem hiding this comment.
it looks like you just took the reshape out of the cross_entropy call? i.e. that's already in the base call
There was a problem hiding this comment.
not quite -- in the original function it has a call to scores.view(num_cands * cands.size(1), -1), but this breaks with BART unless we call reshape. i think it would probably be fine to change the view call to reshape instead in TGA, but this is a slightly more expensive operation (copy instead of view). what do you think?
There was a problem hiding this comment.
since people don't seem to use this functionality too often i'll just change
There was a problem hiding this comment.
yeah given that this is rarely, if ever, used, it'll be better to just make them both reshape
ran bart locally but looks like distillbart, looking into it |
|
@EricMichaelSmith can you take a look at the changes to distillbart when you get a chance? |
parlai/core/torch_generator_agent.py
Outdated
| @@ -864,6 +864,37 @@ def _add_generation_metrics(self, batch, preds): | |||
| """ | |||
| pass | |||
|
|
|||
| def _rank_eval_label_candidates(self, batch, batchsize): | |||
There was a problem hiding this comment.
Maybe this should be a public method?
| @@ -338,6 +341,9 @@ def _perform_forward_passes(self, batch: Batch) -> ForwardPassOutputs: | |||
| mask = self._manipulate_mask( | |||
There was a problem hiding this comment.
nit: maybe mask should now be score_mask to differentiate it from decoder_mask?
yep! tried training for a bit on convai2 and comparing results |
|
Circle CI hanging on self chat tests, I ran locally and they passed |
hmm, it looks like |
Patch description
Abstract Torch Generator Agent candidate ranking ability to a helper function.
Then, override this helper function in BART so that we may delete the score for the start token.
Testing steps
for BART:
for non-BART: