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

Query related to MLPTransformConnector in vae_text #216

Open
varunkumar-dev opened this issue Sep 27, 2019 · 3 comments
Open

Query related to MLPTransformConnector in vae_text #216

varunkumar-dev opened this issue Sep 27, 2019 · 3 comments
Labels
question Further information is requested topic: examples Issue about examples

Comments

@varunkumar-dev
Copy link

Hi,
Thanks for adding vae_text example. I have a question related to MLPTransformConnector. MLPTransformConnector expects an input of size self.encoder.cell.hidden_size*2. I don't understand why it should be multiplied by 2. Shouldn't we just use h of UnidirectionalRNNEncoder as input to MLPTransformConnector instead of (h, c)?

@gpengzhi gpengzhi added the question Further information is requested label Sep 27, 2019
@huzecong
Copy link
Collaborator

Hi, thanks for the issue! Since the example is ported from Texar-TF, I just checked the original code and indeed only h is used. Sorry for this careless mistake, I guess this is because UnidirectionalRNNEncoder in Texar-TF by default doesn't return the cell state while in Texar-PyTorch it does.

However, I feel that having either h or both (h, c) as input doesn't really affect the outcome of the model, but I'm not very sure about this. Does your previous experience with VAEs show that one of the methods is better than the other?

@varunkumar-dev
Copy link
Author

I don't think it will have a big impact on the performance but I think it's worth fixing because someone might use this implementation in a paper. And don't feel bad about it, you guys are doing an amazing job.

@ZhitingHu
Copy link
Member

Thanks for the issue. Using either h or (h, c) combined is legitimate (e.g., different papers implement in different ways). But it'd be good to make it clear in the doc, and probably add the option of using h.

@ZhitingHu ZhitingHu added the topic: examples Issue about examples label Nov 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested topic: examples Issue about examples
Projects
None yet
Development

No branches or pull requests

4 participants