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

Thanks for sharing, but questions remain #1

Closed
shouldsee opened this issue Feb 3, 2022 · 2 comments
Closed

Thanks for sharing, but questions remain #1

shouldsee opened this issue Feb 3, 2022 · 2 comments

Comments

@shouldsee
Copy link

shouldsee commented Feb 3, 2022

Dear devs,

I find this repo simple and smooth to run. However I am confused why you used the same embedding for input and output?

Specifically here in def fowward and in def encode, both used the same reference to self.embedding. This looks weird isnt it? Since source language should use a different encoding when compared to the destination language?

class TransformerTranslator(nn.Module):
    def __init__(self,embed_dim,num_blocks,num_heads,vocab_size,CUDA=False):
        super(TransformerTranslator,self).__init__()
        self.embedding = Embeddings(vocab_size,embed_dim,CUDA=CUDA)
        self.encoder = Encoder(embed_dim,num_heads,num_blocks,CUDA=CUDA)
        self.decoder = Decoder(embed_dim,num_heads,num_blocks,vocab_size,CUDA=CUDA)
        self.encoded = False
        self.device = torch.device('cuda:0' if CUDA else 'cpu')
    def encode(self,input_sequence):
        embedding = self.embedding(input_sequence).to(self.device)
        self.encode_out = self.encoder(embedding)
        self.encoded = True
    def forward(self,output_sequence):
        if(self.encoded==False):
            print("ERROR::TransformerTranslator:: MUST ENCODE FIRST.")
            return output_sequence
        else:
            embedding = self.embedding(output_sequence)
            return self.decoder(self.encode_out,embedding)
@IpsumDominum
Copy link
Owner

Hi @shouldsee , thanks for the issue 👍 :)

Hmm, I think that is a mistake on my part, not sure how the dimensions allowed it to happen. I'll investigate that and get back to you :)

@IpsumDominum
Copy link
Owner

IpsumDominum commented Feb 4, 2022

Newest commit should fix this.

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

No branches or pull requests

2 participants