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

WIP: Basic Lux Integration #169

Closed
wants to merge 3 commits into from
Closed

WIP: Basic Lux Integration #169

wants to merge 3 commits into from

Conversation

willtebbutt
Copy link
Member

@willtebbutt willtebbutt commented May 28, 2024

@yebai this PR adds some basic Lux integration testing. There are some test failures at the minute that I'll patch up -- they'll just be intrinsics + rules etc I imagine.

@avik-pal I've just copied over the Enzyme tests that you pointed us towards, and am converting everything to Float64 (because that's what our testing infrastructure is best equipped to handle at the minute). If you could let me know if anything looks obviously wrong in how I'm making use of the tests, that would be great!

Update: the current failures are all rule-related. I think we basically just need to work through NNlib.jl and make sure that everything there works.

Copy link
Contributor

github-actions bot commented May 28, 2024

Performance Ratio:

┌────────────────────────────┬────────┬─────────┬─────────────┬─────────┐
│                      Label │  Tapir │  Zygote │ ReverseDiff │  Enzyme │
│                     String │ String │  String │      String │  String │
├────────────────────────────┼────────┼─────────┼─────────────┼─────────┤
│                        sum │   41.5 │   0.448 │        3.09 │   0.697 │
│                       _sum │   6.99 │   507.0 │        27.6 │   0.121 │
│                   kron_sum │   80.2 │    3.31 │       205.0 │    23.6 │
│              kron_view_sum │   88.6 │    10.7 │       231.0 │    7.81 │
│      naive_map_sin_cos_exp │    4.1 │ missing │        8.75 │    2.78 │
│            map_sin_cos_exp │   4.63 │    1.71 │        7.46 │    3.39 │
│      broadcast_sin_cos_exp │   4.64 │    2.63 │        1.68 │    2.85 │
│                 simple_mlp │    8.7 │    3.09 │        11.0 │    3.15 │
│                     gp_lml │   16.2 │    4.36 │     missing │ missing │
│ turing_broadcast_benchmark │   6.26 │ missing │        35.2 │ missing │
└────────────────────────────┴────────┴─────────┴─────────────┴─────────┘

@willtebbutt willtebbutt changed the title Basic Lux integration WIP: Basic Lux Integration May 28, 2024
@willtebbutt willtebbutt marked this pull request as draft May 28, 2024 11:14
@willtebbutt willtebbutt mentioned this pull request May 28, 2024
@avik-pal
Copy link

Yeah mostly the needed rules would be same as the ones Enzyme needed https://github.com/FluxML/NNlib.jl/blob/master/ext/NNlibEnzymeCoreExt/NNlibEnzymeCoreExt.jl

@willtebbutt
Copy link
Member Author

I'm closing this in favour of #254 , which addresses all of this.

@willtebbutt willtebbutt closed this Oct 4, 2024
@willtebbutt willtebbutt deleted the wct/lux-testing branch October 4, 2024 10:12
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.

2 participants