neulab / xnmt

eXtensible Neural Machine Translation
Other
185 stars 44 forks source link

File Structure: Models vs. ModelParts #559

Open armatthews opened 5 years ago

armatthews commented 5 years ago

Right now we have two conflicting schemes for choosing which files certain classes belong in. On the one hand we have the modelparts directory, with its decoders, embedders, and attenders. On the other we have the transformer.py file, in which the full model lives.

Does it make sense to move the different parts of the transformer model into the respective modelparts files? There certainly is value to having the transformer implementation in one place, but perhaps this works against the interoperability that xnmt strives for. Perhaps having the transformer example suffices to show how to bring all the model pieces together?

Which structure is best is not clear to me at all. I just wanted to start a discussion on how best to do this going forward.

msperber commented 5 years ago

Yes, we should definitely make things modular and move them to the modelparts package. However, the transformer needs some refactoring because it doesn't use the standard inference mechanism that's used elsewhere in XNMT. On the encoder side, the refactoring is already finished: https://github.com/neulab/xnmt/blob/master/examples/21_self_attention.yaml

The missing step is doing a similar thing for the decoder, including support by the general purpose inference classes etc. After that, we could ideally delete the transformer class (or, if the config file would get too large, we could keep some shortcut definitions in the transformer class, but keep everything else in the modelparts).