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

feat: update ConvMixer to support reactant #1063

Draft
wants to merge 10 commits into
base: ap/reactant_updates
Choose a base branch
from

Conversation

avik-pal
Copy link
Member

@avik-pal avik-pal commented Nov 9, 2024

Copy link

codecov bot commented Nov 9, 2024

Codecov Report

Attention: Patch coverage is 0% with 5 lines in your changes missing coverage. Please review.

Project coverage is 76.31%. Comparing base (51c0e47) to head (fee949f).

Files with missing lines Patch % Lines
src/layers/pooling.jl 0.00% 4 Missing ⚠️
ext/LuxReactantExt/patches.jl 0.00% 1 Missing ⚠️

❗ There is a different number of reports uploaded between BASE (51c0e47) and HEAD (fee949f). Click for more details.

HEAD has 25 uploads less than BASE
Flag BASE (51c0e47) HEAD (fee949f)
46 21
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1063      +/-   ##
==========================================
- Coverage   82.83%   76.31%   -6.53%     
==========================================
  Files         147      145       -2     
  Lines        6072     6049      -23     
==========================================
- Hits         5030     4616     -414     
- Misses       1042     1433     +391     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@avik-pal avik-pal force-pushed the ap/conv_mixer_reactant branch from 090b87e to 23ed44b Compare November 10, 2024 02:01
@avik-pal avik-pal changed the title docs: update ConvMixer to support reactant feat: update ConvMixer to support reactant Nov 10, 2024
@avik-pal avik-pal force-pushed the ap/conv_mixer_reactant branch from db3a12d to ef770a9 Compare November 11, 2024 02:55
Copy link
Contributor

github-actions bot commented Nov 11, 2024

Benchmark Results (ASV)

main a7fb9c9... main/a7fb9c921f0b31...
basics/overhead 0.122 ± 0.0014 μs 0.135 ± 0.005 μs 0.909
time_to_load 0.898 ± 0.0037 s 0.904 ± 0.0085 s 0.994

Benchmark Plots

A plot of the benchmark results have been uploaded as an artifact to the workflow run for this PR.
Go to "Actions"->"Benchmark a pull request"->[the most recent run]->"Artifacts" (at the bottom).

@avik-pal avik-pal force-pushed the ap/conv_mixer_reactant branch 2 times, most recently from 25f13de to ee2fb4f Compare November 15, 2024 02:55
src/layers/pooling.jl Outdated Show resolved Hide resolved
@avik-pal avik-pal force-pushed the ap/conv_mixer_reactant branch 2 times, most recently from b0709d4 to 442bb41 Compare November 15, 2024 23:20
@avik-pal avik-pal force-pushed the ap/conv_mixer_reactant branch 2 times, most recently from a60c1de to 017076f Compare November 27, 2024 03:55
@avik-pal avik-pal force-pushed the ap/conv_mixer_reactant branch 6 times, most recently from dc5e56e to fee949f Compare December 6, 2024 13:07
@avik-pal
Copy link
Member Author

avik-pal commented Dec 6, 2024

Not completely functional yet "error: expects input feature dimension (256) / feature_group_count = kernel input feature dimension (256). Got feature_group_count = 256."

xref: EnzymeAD/Reactant.jl#331

@avik-pal avik-pal force-pushed the ap/conv_mixer_reactant branch 3 times, most recently from 438fede to e3ab45f Compare December 20, 2024 06:36
@avik-pal
Copy link
Member Author

Unfortunately there is a strong discrepancy in the gradients between Zygote and Reactant. Need to debug

@avik-pal avik-pal force-pushed the ap/conv_mixer_reactant branch from d646b75 to a7fb9c9 Compare December 20, 2024 09:53
@avik-pal avik-pal changed the base branch from main to ap/reactant_updates December 20, 2024 09:53
@avik-pal avik-pal force-pushed the ap/conv_mixer_reactant branch 3 times, most recently from 8e1ba6a to 644b60c Compare December 22, 2024 05:04
@avik-pal avik-pal force-pushed the ap/conv_mixer_reactant branch from 9b9f537 to 05500f2 Compare December 22, 2024 07:30
CUDA.allowscalar(false)

@isdefined(includet) ? includet("common.jl") : include("common.jl")

Copy link
Contributor

Choose a reason for hiding this comment

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

[JuliaFormatter] reported by reviewdog 🐶

Suggested change

@avik-pal avik-pal force-pushed the ap/conv_mixer_reactant branch from 71120a2 to d73e9f8 Compare December 23, 2024 08:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant