DOT additions - #392
Conversation
|
The failed test cases seem to be failing trying to rename (remove, actually) firstlevel_brainmask output stream from aamod_firstlevel_epi, as if the scheduler is trying to remove the last output stream twice (the first time correctly removes the epi output stream if/when residuals are not saved). I ran one of these failed test scripts locally on my machine and it passed. Does anyone have a thought what might be going on? Why would the CI runner behave differently than running locally? |
|
I am unsure why checkrequirements runs twice for the same module; however, the easiest fix (and more robust solution anyway) is to tie the residualstreamname to the input (epi or dot), which will not change after removing it from the input. In this case, even if checkrequirements runs again, it will not find it among the outputs. The discrepancy between the CI and your local executions should be investigated, Do you also run the test scripts in the same way as the CI? Is your aa in sync with the central repo (did you merge all changes)? |
I edited the test script to run as a normal script (rather than as a function called by the test utility). So slightly an apples to oranges comparison I suppose.
I always do a pull from upstream before testing my PR locally, so I think my local repo is in sync, but I will double check. In any event, I will modify my changes to tie the residualstreamname to the input and resubmit... |
|
Updates in re why local testing differs from CI runner: It turns out I had an old version of aas_renamestream.m somewhere in my Matlab path that aa was using instead of the repo version. After I deleted this my local testing crashed just like the CI runner (yay!) in re why is removing residuals from aamod_firstlevelmodel crashing? checkrequirements gets called once per subject so any test script that includes more than one subject would eventually try to rename a nonrenameable stream (the first call removes the last output stream, then the next-to-last, and so on) which is why only some of the test scripts failed. This seems like incorrect behavior but the scheduler is venerable and complicated code and I'm not going to dig into it. Instead, I tied residual removal to the input stream name as suggested and pushed it to the PR. It worked locally, hopefully it will now pass all the CI tests. |
|
Incorporated all suggested changes... Thanks for the feedback. |
|
Interesting -- I got an email yesterday that the symlinks CI failed and this morning it says it passed. Did somebody fix something? PS: Don't merge this just yet -- I found one more bug when testing my PR locally that I need to fix... PPS: Related -- Does anyone know what happened to the physreg optional input stream that used to be in aamod_firstlevel_model? This would be useful for our DOT data which often has physiological regressor(s) (e.g., eye tracking, pulse) that could be included in the model. I tested by just adding it back to the header and it it seems to work correctly. When/why did it get removed? |
The test failed only due to some non-specific issue. I have just re-run the test. I am glad it passed this time. |
|
Hello all, (Finally!) converged on what changes we wanted to make to the PR. I have pushed them just now. Assuming this passes the CI, the PR is good to merge. Thanks for your patience, |
2 participants
Hello all,
This PR includes code that begins to properly integrate DOT processing into aa. The changes are mostly making streams renameable so existing modules that work with epi data can be used with DOT data. This is a long-term project; more modules will be modified as we eventually open more aa functionality to DOT. If you're not working with DOT, these changes should be transparent.
There's also aamod_dot_fromnifti that creates a bona fide "dot" stream rather than repurposing "epi" (hence the need for the renameable streams mentioned above) which is how we were previously working with DOT data. Future PR will add aamod_dot_fromsnirf, aamod_dot_fromndot, and so on as time permits.
Also included is a new lightweight QA module that generates a combined carpet, joy, and outlier plot for functional data along with a summary of basic file metrics (dimensions, voxel size, number of NaN, etc). It will work on any 4D data, has no prerequisite streams (the current carpet plot module needs FD and DVARS and segmentation) and saves jpegs that can be loaded using aa_jpeg_crawler and visually reviewed. We have discovered the joy plots are good for our DOT data, which often contain artifacts that are hard to see in a carpet plot.
Finally, aamod_intersubject_correlation has been modified to optionally exclude outliers. These can artificially inflate r values if not removed