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

Graph Eq: Transformers, LayerNorm, Better BN #555

Merged
merged 8 commits into from
Mar 31, 2023
Merged

Graph Eq: Transformers, LayerNorm, Better BN #555

merged 8 commits into from
Mar 31, 2023

Conversation

Giuseppe5
Copy link
Collaborator

No description provided.

@Giuseppe5
Copy link
Collaborator Author

Review done:

  • We now use torch symbolic tracer, and it still traces getitem ops
  • I tested that graph equalization fails when it is not the "self attention" case, and I can confirm it does fail. The good news is we have a way to detect when it is self-attention vs when it is not
  • We could extend to equalize through any generic MHA but it would require some refactoring of _cross_equalize_region, to bridge better the information coming from the graph tracing and the information of the nn.Modules

@Giuseppe5 Giuseppe5 changed the title Graph Eq: MultiheadAttention support Graph Eq: Transformers support Mar 9, 2023
volcacius
volcacius reviewed Mar 13, 2023
View reviewed changes
src/brevitas/graph/equalize.py Show resolved Hide resolved
src/brevitas/graph/equalize.py Outdated Show resolved Hide resolved
src/brevitas/graph/equalize.py Outdated Show resolved Hide resolved
src/brevitas/graph/equalize.py Outdated Show resolved Hide resolved
src/brevitas/graph/equalize.py Outdated Show resolved Hide resolved
src/brevitas/graph/equalize.py Outdated Show resolved Hide resolved
@Giuseppe5 Giuseppe5 force-pushed the eq_mha branch 4 times, most recently from da47053 to bc2ace5 Compare March 14, 2023 13:02
@Giuseppe5 Giuseppe5 force-pushed the eq_mha branch 3 times, most recently from 6003ba9 to 5971d9f Compare March 15, 2023 02:26
@Giuseppe5 Giuseppe5 requested review from volcacius and removed request for volcacius March 15, 2023 09:56
volcacius
volcacius reviewed Mar 20, 2023
View reviewed changes
src/brevitas/graph/equalize.py Show resolved Hide resolved
src/brevitas/graph/equalize.py Show resolved Hide resolved
src/brevitas/graph/equalize.py Outdated Show resolved Hide resolved
tests/brevitas/graph/equalization_fixtures.py Outdated Show resolved Hide resolved
tests/brevitas/graph/test_equalization.py Outdated Show resolved Hide resolved
tests/brevitas/graph/test_equalization.py Outdated Show resolved Hide resolved
volcacius
volcacius reviewed Mar 23, 2023
View reviewed changes
src/brevitas/graph/equalize.py Outdated Show resolved Hide resolved
src/brevitas/graph/equalize.py Outdated Show resolved Hide resolved
src/brevitas/graph/equalize.py Outdated Show resolved Hide resolved
@Giuseppe5 Giuseppe5 force-pushed the eq_mha branch 3 times, most recently from e886304 to 25202c3 Compare March 27, 2023 13:48
@Giuseppe5 Giuseppe5 changed the title Graph Eq: Transformers support Graph Eq: Transformers, LayerNorm, Better BN Mar 27, 2023
@Giuseppe5 Giuseppe5 force-pushed the eq_mha branch 2 times, most recently from 3c076e5 to 61c040e Compare March 29, 2023 11:06
@Giuseppe5
Copy link
Collaborator Author

There's currently a bug for Batch Norm when equalizing for more than 1 iteration

@Giuseppe5
Copy link
Collaborator Author

There's currently a bug for Batch Norm when equalizing for more than 1 iteration

Now fixed

@Giuseppe5 Giuseppe5 force-pushed the eq_mha branch 3 times, most recently from 04a194f to b8710d6 Compare March 31, 2023 11:25
@volcacius volcacius self-requested a review March 31, 2023 13:13
@volcacius volcacius merged commit a8886ac into Xilinx:dev Mar 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@volcacius volcacius volcacius 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.
2 participants
@Giuseppe5 @volcacius