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

Update EnzymeCoreExt.jl #2565

Merged
merged 4 commits into from
Dec 11, 2024
Merged

Update EnzymeCoreExt.jl #2565

merged 4 commits into from
Dec 11, 2024

Conversation

simenhu
Copy link
Contributor

@simenhu simenhu commented Nov 26, 2024

The inactive_noinl failed in broadcast assignment when using Enzyme=0.13.16 and CUDA=5.5.2. Switching to the version that prevents inlining seem to have fixed it. Are there any negative consequences of this?

The issue where the error can be reproduced can be found here: EnzymeAD/Enzyme.jl#2116

@maleadt maleadt requested a review from wsmoses November 26, 2024 12:08
@wsmoses
Copy link
Contributor

wsmoses commented Nov 26, 2024

Can you add your full code as a test?

Copy link

Choose a reason for hiding this comment

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

This code should test the change

@testset "Reverse sum abs2" begin
    x = CuArray([1.0, 2.0, 3.0, 4.0])
    dx = CuArray([0., 0.0, 0.0, 0.0])
    f(x) = sum(abs2.(x))
    Enzyme.autodiff(Reverse, f, Active, Duplicated(x, dx))
    @test all(dx .≈ 2.*x)
end

@simenhu
Copy link
Contributor Author

simenhu commented Nov 28, 2024

Can you add your full code as a test?

Do you still want me to add the code as an test? Assume that you want me to remove the dependence on OrdinaryDiffEq?

@wsmoses
Copy link
Contributor

wsmoses commented Nov 29, 2024

yeah I don't think we should add a dependency here, but if you can make a cuda exclusive test, that would be good to keep

@maleadt maleadt force-pushed the patch-1 branch 2 times, most recently from 4d82710 to 363d900 Compare December 10, 2024 13:36
simenhu and others added 4 commits December 10, 2024 22:34
The inactive_noinl failed in broadcast assignment when using Enzyme=0.13.16 and CUDA=5.5.2. Switching to the version that prevents inlining seem to have fixed it. Are there any negative consequences of this?
Copy link

codecov bot commented Dec 11, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 73.41%. Comparing base (7ff012f) to head (e5e93ba).
Report is 6 commits behind head on master.

Additional details and impacted files
@@             Coverage Diff             @@
##           master    #2565       +/-   ##
===========================================
+ Coverage    9.41%   73.41%   +64.00%     
===========================================
  Files         156      156               
  Lines       14808    14997      +189     
===========================================
+ Hits         1394    11010     +9616     
+ Misses      13414     3987     -9427     

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

@maleadt maleadt merged commit f22c9b4 into JuliaGPU:master Dec 11, 2024
1 check passed
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

Successfully merging this pull request may close these issues.

4 participants