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

Some minor questions regarding the design #29

Closed
XavierXiao opened this issue Jul 21, 2023 · 2 comments
Closed

Some minor questions regarding the design #29

XavierXiao opened this issue Jul 21, 2023 · 2 comments

Comments

@XavierXiao
Copy link

Thanks for updating the implementation frequently! The codebase looks much nicer than when I first looked at it. I took a closer look on the details and would like to ask some questions on specific design choice. Of course I understand that not every specific design comes with a reason, but I would like to know if you have reference/intuition on these things:

  1. It seems like in the main generator, self attention comes before cross attention, while in the upsampler, cross attention comes before self attention.
  2. This line has a residual connection, but it is already done inside the Transformer class. Same here. Is this something new in transformer literature?
  3. Here in the discriminator, the residual is added after attention block. Does it make more sense to add it right after two conv blocks, since the attention block has its own residual connection?
  4. Very tiny issue. The definition in this is unused.
lucidrains added a commit that referenced this issue Jul 21, 2023
@lucidrains
Copy link
Owner

@XavierXiao thank you for the code review! this is more helpful than you could know 🙏

could you see if the latest commit addresses all the issues?

@XavierXiao
Copy link
Author

Yes! Looks good to me. I will let you know if I further spot something.

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