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

Enzyme: add make_zero of cuarrays #2600

Merged
merged 9 commits into from
Dec 23, 2024
Merged

Enzyme: add make_zero of cuarrays #2600

merged 9 commits into from
Dec 23, 2024

Conversation

wsmoses
Copy link
Contributor

@wsmoses wsmoses commented Dec 18, 2024

[only downstream]

@wsmoses wsmoses requested a review from maleadt December 18, 2024 01:39
@maleadt
Copy link
Member

maleadt commented Dec 18, 2024

Enzyme CI fails.

Copy link

codecov bot commented Dec 18, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 9.27%. Comparing base (830c49b) to head (6e5d565).
Report is 5 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #2600       +/-   ##
==========================================
- Coverage   73.51%   9.27%   -64.25%     
==========================================
  Files         157     157               
  Lines       15207   15025      -182     
==========================================
- Hits        11180    1394     -9786     
- Misses       4027   13631     +9604     

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

@wsmoses
Copy link
Contributor Author

wsmoses commented Dec 18, 2024

@maleadt okay fixed!

@maleadt
Copy link
Member

maleadt commented Dec 18, 2024

Still fails. Please have a look at the CI logs.

@wsmoses
Copy link
Contributor Author

wsmoses commented Dec 18, 2024

oh apologies, I just saw the green and assumed it was fine

@maleadt maleadt force-pushed the master branch 15 times, most recently from 5d585c4 to c850163 Compare December 20, 2024 08:18
@maleadt
Copy link
Member

maleadt commented Dec 21, 2024

The last build failed to even precompile the extension:

      From worker 2:	ERROR: LoadError: UndefVarError: `FT` not defined
      From worker 2:	Stacktrace:
      From worker 2:	 [1] top-level scope
      From worker 2:	   @ /var/lib/buildkite-agent/builds/gpuci-15/julialang/cuda-dot-jl/ext/EnzymeCoreExt.jl:583
      From worker 2:	 [2] include
      From worker 2:	   @ ./Base.jl:495 [inlined]
      From worker 2:	 [3] include_package_for_output(pkg::Base.PkgId, input::String, depot_path::Vector{String}, dl_load_path::Vector{String}, load_path::Vector{String}, concrete_deps::Vector{Pair{Base.PkgId, UInt128}}, source::String)
      From worker 2:	   @ Base ./loading.jl:2285
      From worker 2:	 [4] top-level scope
      From worker 2:	   @ stdin:3
      From worker 2:	in expression starting at /var/lib/buildkite-agent/builds/gpuci-15/julialang/cuda-dot-jl/ext/EnzymeCoreExt.jl:2
      From worker 2:	in expression starting at stdin:3

A rebase won't fix this, so cancelling the outstanding builds.

@wsmoses Please verify locally things work first.

@maleadt maleadt marked this pull request as draft December 21, 2024 21:27
@wsmoses
Copy link
Contributor Author

wsmoses commented Dec 21, 2024

Fixed/verified locally

Test Summary: |  Pass  Broken  Total  Time
  Overall     | 27434      11  27445      
    SUCCESS
     Testing CUDA tests passed 

(CUDA) pkg> ^C

julia> 

@wsmoses wsmoses marked this pull request as ready for review December 21, 2024 22:04
@maleadt
Copy link
Member

maleadt commented Dec 22, 2024

Testing now exceeds the 30min limit.

@wsmoses
Copy link
Contributor Author

wsmoses commented Dec 22, 2024

trying it with an hour to see if it's fine

@wsmoses
Copy link
Contributor Author

wsmoses commented Dec 22, 2024

however, it looks like this was already happening previously, so not due to this PR presumably: https://buildkite.com/julialang/cuda-dot-jl/builds/5616#0193de0a-806e-4e36-af34-2abfe9fa437d

@maleadt
Copy link
Member

maleadt commented Dec 22, 2024

Testing still fails

Error in testset extensions/enzyme:
Error During Test at /var/lib/buildkite-agent/builds/gpuci-5/julialang/cuda-dot-jl/test/extensions/enzyme.jl:10
  Got exception outside of a @test
  UndefVarError: `T` not defined
  Stacktrace:
    [1] make_zero!
      @ /var/lib/buildkite-agent/builds/gpuci-5/julialang/cuda-dot-jl/ext/EnzymeCoreExt.jl:593 [inlined]

@@ -246,8 +246,7 @@ steps:
build.message !~ /\[only/ && !build.pull_request.draft &&
build.message !~ /\[skip tests\]/ &&
build.message !~ /\[skip downstream\]/
timeout_in_minutes: 30
soft_fail: true
Copy link
Member

Choose a reason for hiding this comment

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

This is wrong: 6bdb146

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah okay will fix, but also I’m now confused from a high level. There an obvious undef var usage in the patch (which I will fix), which breaks CI but I never hit locally just running test.

is this not run as part of the default tests?

@wsmoses
Copy link
Contributor Author

wsmoses commented Dec 23, 2024

@maleadt okay all properly passes now (and runs in 31mins it seems)

@maleadt maleadt merged commit 972f3f0 into JuliaGPU:master Dec 23, 2024
2 checks passed
THargreaves pushed a commit to THargreaves/CUDA.jl that referenced this pull request Jan 7, 2025
avik-pal pushed a commit to avik-pal/CUDA.jl that referenced this pull request Jan 11, 2025
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.

3 participants