search

CUNY-CL / yoyodyne

Small-vocabulary sequence-to-sequence generation with optional feature conditioning
Apache License 2.0
32 stars 19 forks source link

Concatenated features break symbol decoding #149

Open Adamits opened 1 year ago

Adamits commented 1 year ago

For models where the features are concatenated to the source string, we now handle this in the collator. We simply add the source_token vocabulary length to each feature index in order to avoid clashes: https://github.com/CUNY-CL/yoyodyne/blob/master/yoyodyne/data/collators.py#L71

However, the symbol maps do not track this. Thus, if we want to decode a source (e.g. as a sanity check in the logs), this will not work since the feature indices are out of raneg---they no longer can be meaningfully mapped back to their surface form. Not a critical bug, but definitely an odd behavior.

kylebgorman commented 1 year ago

What would you do about this? My thought was that it's hard to imagine (except for debugging I guess) why you'd want to decode anything but the target.

Adamits commented 1 year ago

Yes I suppose debugging is my only motivation. It is a bit awkward that we have totally untracked values, though.

What would you do about this?

I guess it would essentially require reverting back to the old way of doing this, where we add the features into the source vocab. But iirc that might also cause weird friction.

Otherwise, maybe I could do something tricky in the decode function where I check the source vocab size, and assume any integer > that size should be decoded with the features_vocab at index (idx - source_vocab_size).

I will think through if theres any issues with that.

kylebgorman commented 1 year ago

It occurs to me that clashes can't happen anyways because of the special treatment we give to features. Does that help at all?

Adamits commented 1 year ago

That is a good point. Are you suggesting that we just merge the feature vocab to the source vocab in the setting that they will be concatenated to the source? Or do you mean does it help to justify not making an update?

kylebgorman commented 1 year ago

Yeah, you could just add features to the source vocabulary. (Actually I'd do it regardless of whether concatenation is happening since it's harmless and it's annoying to pass information about whether concatenation is happening---which depends both on whether features are specified and the architecture---into the index and such.)

Adamits commented 1 month ago

Commenting here to say that I am setting a reminder to dig into this. I recently was trying to debug some projects and its quite unpleasant without being able to decode features at runtime.