WashU code changes for merging - #124

I can see that you let SPM handle path settings, but it may be more robust to handle it explicitly with corresponding changes in aap_parameters_defaults.xml and aa_init. See this commit!

I considered setting up the paths in aa_init, but I was thinking aa_init is for common (or even required) third-party stuff like SPM and FSL (I don't imagine the LI toolbox will be used by many aa users). However, I'd be happy to add it if that's the proper way to do things.

I wonder whether you want to apply changes to the PR by adding support to the LI toolbox to aa_init and aap_parameters_defaults.xml.
I have also a few other suggestions:

• One philosophy of aa is to provide an easier-to use interface to the methods, so it would be nicer to parse the command parameters and provide corresponding description in the xml file rather then specifying it as a string-bundle, which means nothing unless you already know the toolbox..
• It would be, perhaps, nicer to save outputs per contrast file (i.e. within the loop) rather then as a bundle. For naming convention, you can look for aamod_firstlevel_contrast
• To 'convert' PS to JPG, you can try opening the PS in a figure and save the figure as jpg

I wonder whether you want to apply changes to the PR by adding support to the LI toolbox to aa_init and aap_parameters_defaults.xml.

If the code checks whether the field is defined in aap before trying to access it (some entries in aa_init already do this) then we don't have to modify the parameter files. Is that not preferable? If a user is not using LI toolbox, they shouldn't have to change their parameter files (which may be highly customized for their particular installation). We've gotten burned by this, where we recently downloaded a new repo, and aa stopped working because it was suddenly looking for a "VBQ toolbox" which we have don't have, don't need, and have no intention of using.

so it would be nicer to parse the command parameters and provide corresponding description in the xml file rather then specifying it as a string-bundle, which means nothing unless you already know the toolbox.

Probably a better solution -- I was trying to solve a problem for us quickly and the LI interface is just weird (it takes a parameter string, not individual parameters, as input). I'll look into this.

It would be, perhaps, nicer to save outputs per contrast file (i.e. within the loop) rather then as a bundle. For naming convention, you can look for aamod_firstlevel_contrast

I will look into this.

To 'convert' PS to JPG, you can try opening the PS in a figure and save the figure as jpg

Do you mean do this in the aamod_LI reporting section? That would be a nice solution, but unless I'm mistaken, Matlab can't read PS files (it can write them but not read them). Ergo changing the LI toolbox to save as JPEG rather than PS. It's not an elegant solution, but I can't think of any way around it.

However, you can use linux “gs” to convert ps to jpg

Do you have any tips for this? I played with ghostscript for an entire afternoon a few weeks back and my ps-to-jpg conversion results always looked terrible (which is why I gave up on the idea).

Also, this approach would assume gs is installed. That may be true for Linux, but it isn't for OS X.

Have you tried the example I provided? gs -sDEVICE=jpeg -o test.jpg test.ps
The code would run on the cluster which is linux in most cases. Besides, you can always test first whether gs is installed (see https://github.com/rhodricusack/automaticanalysis/blob/master/aa_engine/aas_findinputstreamsources.m, lines 179-181).

Hi Tibor,

I finally found some time to make changes you recommended. I reworked the LI options as separate named parameters in the header. I now check for ps and try to do a postscript to jpeg conversion (assuming ghostscript is installed) during reporting. If conversion fails, I print an informative message to the user on how to correct the problem. Also the module is now session domain.

I didn't make an entry in aa_init for the module because this is an spm toolbox and it automatically gets added to the path during spm initialization. As such, another entry in the aap structure would be superfluous.

I pushed this to my forked repo -- the Git documentation seems to indicate it will get automatically added to this PR. Is there anything else I need to do?

Thanks for your help,
Mike

Hi Tibor,

Finally got around to implementing your recommendations:

1. split the interface as headers aamod_firstlevel_LI.xml and aamod_secondlevel_LI.xml (both assumed to take a t- or F-map, because that is what the LI toolbox is designed to work on) and tweaked the mfile so that it works at either subject or study level.

2. added an explicit initialization to aa_init. This isn't necessary (aa will find the LI toolbox as long as it's installed in SPM/toolbox) but if this helps people then great!

3. Sign flipped the negative threshold parameter in the code so now all parameters the user interacts with are positive.

I think that's everything. I commit'ed and pushed upstream; I think the changes should show up in the PR automatically.

Mike

I am terribly sorry for my delay, but I am engaged in implementing aa in Norwich. Unfortunately, their cluster uses X11-forwarding rather than VNC, therefore I have to stay connected to their VPN to keep the pipeline rolling which means I cannot connect to other networks (i.e. ours at the RHUL) to do other works, such as testing this PR.
I promise I try to find some spare nanoseconds :) some time to resolve this PR.

Hi Tibor,

No worries. These days I think we all have more to do than hours in the day :-)

I have tested the PR and it is working. I am happy to merge if there is no objection.
I wait until tomorrow.

Well, I don't have any objections. :-)

Thanks much,
Mike