Repository navigation
Error when specific subjects are missing specific sessions. #401
Description
Hi all, this PR by @jones-michael-s and this (a5baa45) commit has broken aa for me.
Particularly, if a session doesn't exist for a subject, I was marking it using an empty array and that was dependent on the current session index.
However, since the new code uses the first index and if a subject misses the very first session but has others available, I get an error because series{1} doesn't satisfy any of those conditions (whereas series{sess}) did before.
Activity
tiborauer commentedon Nov 14, 2024
Could you write a corresponding test we could add to the CI?
TamerGezici commentedon Nov 14, 2024
Sure, I can. But the data is kind of complex, and I'll have to look at the test data to see that.
My data is like this: I have 55 subjects that have done 3 different tasks, (consisting of 2-3 sessions) and I process them all together, so if a subject did not do task A, they are marked with an empty list. And the other available sessions are incorporated.
I am not very well-versed with setting up tests, could @jones-michael-s take a look at this?
For now, I am using an older local version of aa. I ran into this while I was setting up aa for an intern.
TamerGezici commentedon Jan 23, 2025
@jones-michael-s
Have you had an opportunity to check this?
@tiborauer I don't think it is possible setup a test for this with the given ds000114 dataset we are using for our tests. Because all subjects have completed all tasks.
That is because my dataset is like this:
When I put subjects and sessions into aa, I was marking the missing sessions (ie., for task B subjects which have not done task A) with an empty list. This is a work-around which you had suggested to me before here: #366 (comment). But this no longer works due to the changes above.
How should I proceed?
TamerGezici commentedon Jan 23, 2025
Also, I just noticed that what Mike changed in that commit was a change I had implemented 2.5 years ago. The discussion was covered over here:
#331 (comment)
So, I think maybe it was me who broke it in the first place.
I wonder if there is a possible solution that would allow us to keep both solutions at the same time, somehow? When I eye-ball the thing though it seems like my "fix" is a better one. Why check the first index, instead of checking the sess? Maybe Mike's use-case was different and this was leading to some error so he had to revert that change?