Skip to content

Fixes for Windows - #2206

Merged
maleadt merged 3 commits into
masterfrom
tb/windows
Dec 21, 2023
Merged

maleadt merged 3 commits into
masterfrom
tb/windows

Conversation

@maleadt

@maleadt maleadt commented Dec 21, 2023

Copy link
Copy Markdown
Member

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
@codecov

codecov Bot commented Dec 21, 2023

Copy link
Copy Markdown

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
@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