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.

Sign up for GitHub

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

Jump to bottom

Add causal self-attention block #218

Merged
merged 6 commits into from
Mar 13, 2023
Merged

Add causal self-attention block #218

merged 6 commits into from
Mar 13, 2023

Conversation

Warvito
Copy link
Collaborator

@Warvito Warvito commented Feb 3, 2023

Fixes #196

@Warvito
Add original SABlock
35e8c0f 
Signed-off-by: Walter Hugo Lopez Pinaya <ianonimato@hotmail.com>
@Warvito Warvito linked an issue Feb 3, 2023 that may be closed by this pull request
Add self-attention block for autoregressive transformer #196
Closed
@Warvito
Move towards implementation without eionps
06314aa 
Signed-off-by: Walter Hugo Lopez Pinaya <ianonimato@hotmail.com>
@Warvito
Move towards implementation without eionps
02cd120 
Signed-off-by: Walter Hugo Lopez Pinaya <ianonimato@hotmail.com>
@Warvito
Copy link
Collaborator Author

Warvito commented Feb 3, 2023 •
edited

Based on https://github.com/pytorch/pytorch/blob/master/torch/nn/modules/transformer.py, https://github.com/karpathy/nanoGPT/blob/master/model.py, https://github.com/karpathy/minGPT/blob/master/mingpt/model.py, and https://github.com/huggingface/transformers/blob/main/src/transformers/models/gpt2/modeling_gpt2.py

@Warvito
Merge branch 'main' into 196-add-self-attention-block-for-autoregress…
b3797d6 
…ive-transformer
@Warvito
Add causal operation to self attention block
f50b9ee 
Signed-off-by: Walter Hugo Lopez Pinaya <ianonimato@hotmail.com>
@Warvito Warvito marked this pull request as ready for review March 12, 2023 11:59
marksgraham
marksgraham approved these changes Mar 13, 2023
View reviewed changes
Copy link
Collaborator

@marksgraham marksgraham left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, just a fix to the import and typing

generative/networks/blocks/selfattention.py Outdated
else:
self.mask = None

def forward(self, x):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typing missing from forward

tests/test_selfattention.py
from monai.networks import eval_mode
from parameterized import parameterized

from generative.networks.blocks.selfattention import SABlock
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think we should use an __init__.py file so we can have the nicer
from generative.networks.blocks import SABlock?

@Warvito
Address comments
96e9c9d 
Signed-off-by: Walter Hugo Lopez Pinaya <ianonimato@hotmail.com>
@Warvito Warvito merged commit e135f42 into main Mar 13, 2023
@Warvito Warvito deleted the 196-add-self-attention-block-for-autoregressive-transformer branch March 18, 2023 19:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@marksgraham marksgraham marksgraham approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add self-attention block for autoregressive transformer
2 participants
@Warvito @marksgraham