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

Fixes for Windows #2206

Merged
merged 3 commits into from
Dec 21, 2023
Merged

Fixes for Windows #2206

merged 3 commits into from
Dec 21, 2023

Conversation

maleadt
Copy link
Member

@maleadt maleadt commented Dec 21, 2023

Among other things, reverts #2109 (cc @utkarsh530). Turns out that broadcast's (ab)use of Ref boxes for scalar arguments is a problem: Ephemeral objects like that get freed quickly, leading to illegal memory accesses on the GPU. We could root all arguments, but that would make launching kernels more expensive. I'm inclined to just not support a use case like that, at least for now. If people care greatly about this, please comment in #267. For now, this should fix FluxML/Zygote.jl#1473.

Turns out broadcast uses ephemeral Ref boxes to pass scalars,
which get freed rapidly and can lead to illegal memory accesses
when the broadcast kernel accesses them.
@maleadt maleadt added the bugfix This gets something working again. label Dec 21, 2023
Copy link

codecov bot commented Dec 21, 2023

Codecov Report

Attention: 2 lines in your changes are missing coverage. Please review.

Comparison is base (05959ad) 72.97% compared to head (610e5df) 72.95%.

Files Patch % Lines
lib/nvml/device.jl 66.66% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2206      +/-   ##
==========================================
- Coverage   72.97%   72.95%   -0.02%     
==========================================
  Files         159      159              
  Lines       14656    14648       -8     
==========================================
- Hits        10695    10687       -8     
  Misses       3961     3961              

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

@maleadt maleadt merged commit f185f79 into master Dec 21, 2023
1 check passed
@maleadt maleadt deleted the tb/windows branch December 21, 2023 17:54
LiorSinai added a commit to LiorSinai/TransformersLite.jl that referenced this pull request Dec 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix This gets something working again.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Pullback on mean() gives illegal memory access code 700
1 participant