-
Notifications
You must be signed in to change notification settings - Fork 25
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
Specialize adapt_structure for small tuples #81
Specialize adapt_structure for small tuples #81
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #81 +/- ##
==========================================
- Coverage 93.65% 92.85% -0.80%
==========================================
Files 6 6
Lines 63 70 +7
==========================================
+ Hits 59 65 +6
- Misses 4 5 +1 ☔ View full report in Codecov by Sentry. |
Can't you do this with a simple This also reminds me of JuliaLang/julia#53665. |
I was going to suggest that we could use an if-statement, is that your preference? |
Yes, you're now putting additional pressure on the dispatch system where the compiler should be able to handle this just fine. |
bump 🙂 |
I don't like that we don't have a test for this, but OK I guess. Seeing how you went with the |
Thank you for merging! Yeah, I agree about the tests. I did try to write a simple reproducer, but I wasn't able to recreate the failure. I'm not sure what ingredients are needed. I'll take another look to see if I can cook something up. I tend to try to avoid using Julia internals, but I can check to see if |
Yeah, that's fair. |
I've updated, and I'll try to continue updating/simplifying, the reproducer in #79. It's drastically simpler now, although there's still a lot more to go. |
Thanks again for fixing this! This fixed 955 inference failures per timestep 😬 found by JET! 🚀 |
Closes #79.