Allow separate cardiac and resp files in BIDS format #167
Conversation
Release v5.1.2
Solving #164 : Made the import of the .tsv files possible with different sampling rates for cardiac and resp signals. I used linear interpolation to replace the NaNs and upscaled the respiratory signal, replicating what happens when using the .log files. The output confound matrix is very similar to what the toolbox returns with the .log files.
|
Dear Alex, Thank you so much for providing this improvement. I will try to include it into the next PhysIO release (probably in March). In order to do so, I would like to create an integration test that checks the results of this BIDS reader. This would need your two separate BIDS input files and an output Would you be able to share this data with me? Furthermore, could you please also add a version of the Contributor License Agreement file to this pull request that has a table row with your name and info inserted. I have explained the rationale for that here and here. Thank you very much again and all the best, |
|
Hello Lars, Sorry for the delay, here are the files you requested. All the best, |
|
Great, thank you, Alexandre! I will get back to you when I have setup the test. All the best, |
|
Merging into development for unit testing |
|
Dear Alexandre @alexsayal, I have now created an intergration test using your logfiles and saved results, but there are some small discrepancies between those outputs and the runs on my machine that I am trying to track down. Do you happen to know with which version of PhysIO you ran the tests? I deducted it has to be before You can check this by running Thank you and all the best, |
|
Hello @mrikasper , Something very strange happened, the physio.mat I sent was in fact generated using version Anyway, I re-send the output, now sure I am using the latest version. Sorry for this. |
|
Dear @alexsayal, Thank you for getting back to me so swiftly! And no worries, that's exactly why I am writing the tests :-) What could have happened is that the PhysIO version used with SPM was still in the path (the one you have to put in One more thing: I have to check the files you sent, but one thing I noticed when I was rerunning on my machine was that the cut-out of unused data was at the end of the whole time series before (see screenshot with Thank you for your help and continued input,
|
|
Hello @mrikasper , Yes, cutting-out tics at the beginning of the acquisition is the correct approach. I have verified this on the original .log files as well, since the |
|
Dear Alexandre, Great, thank you so much for checking and sending the updated results - I am happy to report the new integration tests run through here on my machine with identical results, so we can include your improvements into the next release. One more question: I think the direct comparison with the Siemens log is a useful additional cross-over test, so that we can check consistency between the two read-ins. Would you be willing to share the Thank you once again and all the best, |
|
Dear @mrikasper, I am glad to hear that. Of course, I attach the Physio_20190114_145703_d056ae5f-9ba2-4a7b-8288-838f01ea258d_RESP.log All the best, |
|
Thank you, @alexsayal, that's great! I will incorporate it into the testing framework, but maybe it only makes it into the release after next. All the best, |
Allow separate cardiac and resp files in BIDS format
Solving #164 :
Made the import of the .tsv files possible with different sampling rates for cardiac and resp signals. I used linear interpolation to replace the NaNs and upscaled the respiratory signal, replicating what happens when using the .log files.
The output confound matrix is very similar to what the toolbox returns with the .log files.