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

Indxing type stability #137

Open
rafaqz opened this issue Jan 21, 2024 · 5 comments
Open

Indxing type stability #137

rafaqz opened this issue Jan 21, 2024 · 5 comments

Comments

@rafaqz
Copy link
Collaborator

rafaqz commented Jan 21, 2024

We need to thoroughly audit type stability when indexing, maybe with some @inferred tests.

See: #131

@Alexander-Barth
Copy link
Contributor

I think it would be good to avoid this if statement here using dispatch:

if any(is_batch_arg, i)

@meggart
Copy link
Owner

meggart commented Feb 15, 2024

From my tests, everything should be type stable now with #146 . However, we don't have formal unit tests for this yet. @rafaqz I have never written type-stability tests, do you have a few pointers?

@Alexander-Barth I know it would be great to get rid of any runtime checks. However, some things are only detectable at runtime, for example if a vector index like [7,5,6,4,1,2,3] is dense or not and can be replaced by a single unit range or has to be split up. However, we have organized the code so that all return types should definitely be infered.

@rafaqz
Copy link
Collaborator Author

rafaqz commented Feb 15, 2024

There are a few ways. @test @inferred can help but wont always pick up internal type stability problems that never escape the function. Checking @test (@allocations f(x)) == 0 can help with that, and maybe using something larger than zero if you know how many allocations there should be. Type instability allocates a lot.

@rafaqz
Copy link
Collaborator Author

rafaqz commented Feb 15, 2024

Theres also @test_call in JET but it often returns too many things to be useful

https://aviatesk.github.io/JET.jl/dev/jetanalysis/#JET.@test_call

@meggart
Copy link
Owner

meggart commented Feb 20, 2024

For now I added some @test @inferred tests in the unit tests to make sure that type instabilities never leak out of the functions and this already helped detecting a few branches that were still type-unstable. I would suggest that more testing with JET can be added later when problems arise.

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

No branches or pull requests

3 participants