Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add option to concatenate features for pointer generator #136

Open
Adamits opened this issue Aug 17, 2023 · 3 comments
Open

Add option to concatenate features for pointer generator #136

Adamits opened this issue Aug 17, 2023 · 3 comments
Assignees
Labels
enhancement New feature or request

Comments

@Adamits
Copy link
Collaborator

Adamits commented Aug 17, 2023

I was just thinking, though our pointer-generator implementation(s) take care to encode features separately so that they are not used in the attention distribution for the pointer probabilities, I think it is worth making it easy to just consider features as other input symbols along with the lemmas.

That is, to concatenate the features with the input just like we do for the 'vanilla' seq2seq models. This is just for comparison as sometimes these models learn things on their own without much intervention.

@kylebgorman
Copy link
Contributor

Sure, I have been wondering that too. Is there any way to absolutely forbid them from being copied into the target sequence though? (And wouldn't that give rise to a decoding error if they were bceause their indices would not be in the relevant index?)

Here's my hypothesis though (and maybe this is just my "neat"ness coming out in this admittedly "scruffy" project): concatenation is never substantially better than separate encoders, and it's occasionally worse. If I am right about this, then we could free ourselves of all that code specific to concatenating models, eventually.

@kylebgorman kylebgorman added the enhancement New feature or request label Aug 17, 2023
@Adamits
Copy link
Collaborator Author

Adamits commented Aug 17, 2023

This is a good point. If we implement the option to concat for every model, then we could run this experiment.

@kylebgorman
Copy link
Contributor

"To conatenate or not to concatenate": great SIGMORPHON short paper idea.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants