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

fix: bypass segfault with fill complex #245

Merged
merged 3 commits into from
Nov 8, 2024
Merged

fix: bypass segfault with fill complex #245

merged 3 commits into from
Nov 8, 2024

Conversation

avik-pal
Copy link
Collaborator

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

fixes #244

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Reactant.jl Benchmarks

Benchmark suite Current: e4d6956 Previous: f34ec8f Ratio
ViT base (256 x 256 x 3 x 32)/forward/CPU/Reactant (optimize = :after_enzyme) 5942876743 ns 7027955167 ns 0.85
ViT base (256 x 256 x 3 x 32)/forward/CPU/Reactant 5395404122 ns 5911790203 ns 0.91
ViT base (256 x 256 x 3 x 32)/forward/CPU/Reactant (optimize = :before_enzyme) 5269258358 ns 5039003346 ns 1.05
ViT base (256 x 256 x 3 x 32)/forward/CPU/Reactant (optimize = :only_enzyme) 7597834595 ns 7620308614 ns 1.00
ViT base (256 x 256 x 3 x 32)/forward/CPU/Lux 33357751058 ns 31357074001 ns 1.06
ViT small (256 x 256 x 3 x 4)/forward/CPU/Reactant (optimize = :after_enzyme) 1741402413 ns 1572787384 ns 1.11
ViT small (256 x 256 x 3 x 4)/forward/CPU/Reactant 1613689556 ns 1548995246 ns 1.04
ViT small (256 x 256 x 3 x 4)/forward/CPU/Reactant (optimize = :before_enzyme) 1583556152 ns 1555509740 ns 1.02
ViT small (256 x 256 x 3 x 4)/forward/CPU/Reactant (optimize = :only_enzyme) 3373820408 ns 3467552492 ns 0.97
ViT small (256 x 256 x 3 x 4)/forward/CPU/Lux 2685295180 ns 3419731143 ns 0.79
ViT tiny (256 x 256 x 3 x 32)/forward/CPU/Reactant (optimize = :after_enzyme) 2190548924 ns 2180418814 ns 1.00
ViT tiny (256 x 256 x 3 x 32)/forward/CPU/Reactant 2209486922 ns 2173460673 ns 1.02
ViT tiny (256 x 256 x 3 x 32)/forward/CPU/Reactant (optimize = :before_enzyme) 2216823423 ns 2158353524 ns 1.03
ViT tiny (256 x 256 x 3 x 32)/forward/CPU/Reactant (optimize = :only_enzyme) 4088922391 ns 3927830322 ns 1.04
ViT tiny (256 x 256 x 3 x 32)/forward/CPU/Lux 6102836387 ns 6253378741.5 ns 0.98
ViT tiny (256 x 256 x 3 x 4)/forward/CPU/Reactant (optimize = :after_enzyme) 1452811113 ns 1440134479 ns 1.01
ViT tiny (256 x 256 x 3 x 4)/forward/CPU/Reactant 1452819092 ns 1421557207 ns 1.02
ViT tiny (256 x 256 x 3 x 4)/forward/CPU/Reactant (optimize = :before_enzyme) 1459665876 ns 1428360591 ns 1.02
ViT tiny (256 x 256 x 3 x 4)/forward/CPU/Reactant (optimize = :only_enzyme) 3312061992 ns 3208488833 ns 1.03
ViT tiny (256 x 256 x 3 x 4)/forward/CPU/Lux 1239021104 ns 1139085595.5 ns 1.09
ViT tiny (256 x 256 x 3 x 16)/forward/CPU/Reactant (optimize = :after_enzyme) 1765396323 ns 1712651271 ns 1.03
ViT tiny (256 x 256 x 3 x 16)/forward/CPU/Reactant 1757736521 ns 1706763760 ns 1.03
ViT tiny (256 x 256 x 3 x 16)/forward/CPU/Reactant (optimize = :before_enzyme) 1754664859 ns 1704312368 ns 1.03
ViT tiny (256 x 256 x 3 x 16)/forward/CPU/Reactant (optimize = :only_enzyme) 3560076766 ns 3479746822 ns 1.02
ViT tiny (256 x 256 x 3 x 16)/forward/CPU/Lux 3497464739.5 ns 3154626911 ns 1.11
ViT small (256 x 256 x 3 x 16)/forward/CPU/Reactant (optimize = :after_enzyme) 2232287745 ns 2175602283 ns 1.03
ViT small (256 x 256 x 3 x 16)/forward/CPU/Reactant 2219994004 ns 2162683110 ns 1.03
ViT small (256 x 256 x 3 x 16)/forward/CPU/Reactant (optimize = :before_enzyme) 2213710848 ns 2187544254 ns 1.01
ViT small (256 x 256 x 3 x 16)/forward/CPU/Reactant (optimize = :only_enzyme) 4039640825 ns 3928792736 ns 1.03
ViT small (256 x 256 x 3 x 16)/forward/CPU/Lux 6424706055 ns 6172504320 ns 1.04
ViT small (256 x 256 x 3 x 32)/forward/CPU/Reactant (optimize = :after_enzyme) 3069515063 ns 2997646442 ns 1.02
ViT small (256 x 256 x 3 x 32)/forward/CPU/Reactant 3063561681 ns 2957532725 ns 1.04
ViT small (256 x 256 x 3 x 32)/forward/CPU/Reactant (optimize = :before_enzyme) 3077679926 ns 2960803098 ns 1.04
ViT small (256 x 256 x 3 x 32)/forward/CPU/Reactant (optimize = :only_enzyme) 5002763260 ns 4863944327 ns 1.03
ViT small (256 x 256 x 3 x 32)/forward/CPU/Lux 22494033295 ns 22191688400 ns 1.01
ViT base (256 x 256 x 3 x 16)/forward/CPU/Reactant (optimize = :after_enzyme) 3251101343 ns 3157390991 ns 1.03
ViT base (256 x 256 x 3 x 16)/forward/CPU/Reactant 3287250300 ns 3212937029 ns 1.02
ViT base (256 x 256 x 3 x 16)/forward/CPU/Reactant (optimize = :before_enzyme) 3299074516 ns 3419812778 ns 0.96
ViT base (256 x 256 x 3 x 16)/forward/CPU/Reactant (optimize = :only_enzyme) 5287795368 ns 5010112071 ns 1.06
ViT base (256 x 256 x 3 x 16)/forward/CPU/Lux 12858711128 ns 10940648930 ns 1.18
ViT base (256 x 256 x 3 x 4)/forward/CPU/Reactant (optimize = :after_enzyme) 2049303366 ns 1843742771 ns 1.11
ViT base (256 x 256 x 3 x 4)/forward/CPU/Reactant 1984085016 ns 1827524417 ns 1.09
ViT base (256 x 256 x 3 x 4)/forward/CPU/Reactant (optimize = :before_enzyme) 1887141061 ns 1842645194 ns 1.02
ViT base (256 x 256 x 3 x 4)/forward/CPU/Reactant (optimize = :only_enzyme) 3740943891 ns 3593953850 ns 1.04
ViT base (256 x 256 x 3 x 4)/forward/CPU/Lux 3469995214 ns 3544018917 ns 0.98

This comment was automatically generated by workflow using github-action-benchmark.

@mofeing
Copy link
Collaborator

mofeing commented Nov 8, 2024

great! can you add a smaller test in "complex.jl" like the following? it's the simplest case we can test this with

x = 1.0 + 2.0im
y = Reactant.ConcreteRNumber(x)

f = Reactant.compile((y,)) do z
		z + Reactant.promote_to(TracedRNumber{ComplexF64}, 1.0 - 3.0im)
end

@test isapprox(f(y), 2.0 - 1.0im)

(it could be simpler but parameter-less functions are broken #196)

@avik-pal
Copy link
Collaborator Author

avik-pal commented Nov 8, 2024

great! can you add a smaller test in "complex.jl" like the following? it's the simplest case we can test this with

done

@mofeing
Copy link
Collaborator

mofeing commented Nov 8, 2024

mmm I think you broke control flow?

@avik-pal
Copy link
Collaborator Author

avik-pal commented Nov 8, 2024

oops... let me check

@avik-pal avik-pal requested a review from mofeing November 8, 2024 15:43
@mofeing mofeing merged commit 1ff11c9 into main Nov 8, 2024
20 of 30 checks passed
@mofeing mofeing deleted the ap/fix_segfault branch November 8, 2024 16:16
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.

mlirDenseElementsAttrSplatGet with ComplexNumbers lead to a SegFault
2 participants