split up tests #1868
split up tests #1868
Conversation
test/arrays.jl
Outdated
| sc = rand(Float32, 6) | ||
| bi = rand(Float32, 6) | ||
| #WARN: this is where there is a segfault that I can't figure out | ||
| #Enzyme.autodiff(Reverse, bc2_loss_function, Active, Duplicated(x, Enzyme.make_zero(x)), |
There was a problem hiding this comment.
I have explicitly confirmed that in isolation bc2_loss_fuction works fine on the provided arguments, but this autodiff segfaults. Becoming more confident this is an enzyme bug. Not sure what was preventing it from segfaulting before this PR.
|
open a mwe issue for each of these? |
|
I think we need to fix the segfault before this goes in. Unfortunately I can't reproduce locally. Can you setup a tmate instance with it on it or something, and send me via slack |
|
Ok. I don't think I've ever let the segfaulting test run in CI/CD, let's see what that does first, I've just uncommented it. |
|
@ExpandingMan given tests constantly going in an out, I think this is probably too invasive/difficult to do in one go. Can you do this as PR's with one file move at a time? [that way the conflicts are less likely and it'll be easier to review and confirm] |
|
Frankly that could take forever, there is quite a lot of stuff to get split up. I'd like to get it in, but if you think it's going to be that difficult, I'll leave it to you (if this is indeed something you feel is worth doing). Even if I did this piecemeal I wouldn't know when to do what. |
|
maybe do like 3-4 in a first pass (and move out utils)? |
|
Could we maybe just try to keep track of which tests are added between the PR going up and it being merged? Would it really be that hard? |
|
I mean sure if you want, but also bear in mind that moving everything together means it'll take longer to review.....which means another test PR might get in before then....necessitating another review... |
This is a re-do of #1824 for post 0.13.
To reiterate, the goal here is to roughly group the tests by the functionality they are testing and separate them into a large number of files which should be possible to run entirely independently. Again the way I have grouped these tests may not always make a lot of sense, it seems likely they'll get shuffled around more in the future (or before this gets merged) but this seems like a decent start.
There is still a major issue that I've so far been unable to resolve: a setfault in
arrays.jl, which I've seen before when trying to split up the tests. It sure looks to me like the fact that it segfaults is an enzyme bug, as I don't see any reason it should do that, especially merely as a result of being in a different file.As of writing, everything else in here passes as expected for me (x86 linux).