bbfrederick/rapidtide

--acfix in rapidtide does not work

themeo opened this issue · 7 comments

themeo commented

Describe the bug
After running rapidtide from a singularity container (2.6.0), I get the following error:

Pass number 1
checking reference regressor autocorrelation properties
searching for sidelobes with amplitude > 0.1 with abs(lag) < 30.0 s

WARNING: check_autocorrelation found bad sidelobe at 10.414616713318386 seconds (0.09601889608872338 Hz)...
Removing sidelobe
subjecting lag times to modulus
removing spectral component at sidelobe frequency
Traceback (most recent call last):
File "/usr/local/miniconda/bin/rapidtide", line 23, in
rapidtide_workflow.rapidtide_main(rapidtide_parser.process_args(inputargs=None))
File "/usr/local/miniconda/lib/python3.11/site-packages/rapidtide/workflows/rapidtide.py", line 1528, in rapidtide_main
transferfunc=optiondict["transferfunc"],
~~~~~~~~~~^^^^^^^^^^^^^^^^
KeyError: 'transferfunc'

To Reproduce
This is my call:
singularity exec /home/language/jaksze/rapidtide-2.6.0.simg rapidtide epi.nii.gz output --filterband lfo --passes 3 --nprocs 24 --noprogressbar --acfix

I know it is an experimental option, but I found some other experimental options having a positive impact on the SNR, so I tried also this one.

bbfrederick commented

When I say an option is experimental, I mean that it may or may not be beneficially scientifically or might not work as intended - it's not a license to crash! In any case, it's not that experimental - I've found it beneficial - I should move it out of that section.

I found and (I think) fixed the bug - I phased out specifying the transfer function for the filter a while back, and I guess I missed that call. I removed the offending line, and that should fix it, but I don't seem to have any data lying around with problematic sidelobes. I'll push this as 2.6.1 - the new container will probably be available on Dockerhub in about a half hour (the container takes about 20 minutes to build and push).

themeo commented
bbfrederick commented

Container is building now.

themeo commented
bbfrederick commented

Glad to hear it!

About computation time - I wouldn't think it would take THAT much longer, since this is just a filtering operation when preparing the regressor - that's not a very computationally intensive step. Although maybe it's doing a lot more despeckling now, which can up the processing time. If you include a runtimings.txt file with and without, we could see where it's spending its time.

themeo commented

You are right, when I reran it, the difference was only 17% longer when including --acfix.

bbfrederick commented

Glad it worked!