new aa_test and developer tools - #240
Conversation
|
@hhentschke, It might be interesting to you. |
|
I can see that some changes are based on previous versions. Please, merge the latest version of automaticanalysis/automaticanalysis. E.g.: aa_engine/aas_runfslcommand.m does not have commas and does not use the spm interface. I really like the examples/aa_example_crawler.m. Can you rewrite it in a way that it re-creates the examples/README.md by adding the headers of examples to it? You could consider writing a developer/aa_developer_crawler.m, too. |
|
I updated the PR to include the latest aas_runfslcommand. Changing aa_example_crawler to generate a README is a bit problematic. I went the path of least resistance when I wrote the function, simply looping over the files and doing a "help ". As such, Matlab "help" command does all the heavy lifting. The problem is I don't see a way to get the text "help" returns in order to format it into a Markdown file. It's a good suggestion, but perhaps we could save it for a future update? |
|
@jones-michael-s, could you, please, make sure you merge the current master into your branch? Then I can start testing and merge. |
|
Is there a clever way of checking that my PR is (or is not) up-to-date with current master (besides doing a pull upstream / push master)? I fear I have an approximately eleventy million percent chance of screwing this up. |
|
The clever way is the fetch/merge. Whatever you did (committed) since the last merged PR should be still there. If there is any conflict between the central master and your aa_test branch, they will be marked and you can handle them one-by-one. If you are afraid of screwing up, you can first create a new (sandbox) branch based on your aa_test branch (a copy) and fetch/merge the central master into it. If anything goes wrong, you can simply delete the (sandbox) branch. Another, more tedious and error-prone way is to go through your PR itself. Check whether the proposed changes in files are intended. That is how I eyeballed the (unintended) change in |
|
Okay, I did a fetch/merge and made sure aa_test branch is up to date with master. Everything looks good to go (you were correct to be concerned, but the aas_runfslcommand.m thing was caused by a last-minute addition to the original PR and turned out to be an isolated glitch.) Anyway, thanks for the help. |
|
Finally had a chance to look over your edits on my PR. I don't understand why you restructured However, a larger problem is Am I misunderstanding something? |
|
|
I agree with everything in 2, however in the interest of finishing up this PR perhaps we could postpone these changes to aas_processBIDS, especially if we end up moving to BIDS-MATLAB. Additionally, it might play better with current aa users if API changes don't happen between major revisions. |
|
Sure. I would not touched |
|
I'll take care of it (i need the git practice anyway). |
2 participants
Hello all,
This PR adds a "developer" directory to the repo including tools for easier PR testing. The workhorse is aa_test.m, which is a retooled version of the previous aatest.m that lived in /aa_testcode (/aa_testcode has been deleted). The new PR testing scripts live in /developer/testscripts.
Previously, we used the scripts in /examples to test PR. However, example scripts and PR testing scripts are different animals. An example script should be an extensively commented mfile/xml pair that helps get a user up to speed with aa or a new task in aa. A testing script should be efficient code for exercising a cross section of code in the shortest amount of time. An example script may require customization for a specific user's data. A test script should run across multiple installations as-is.
By separating example scripts and PR testing scripts in the repo, we can make both better. That is what the new /developer directory tries to accomplish.
There's a comment block at the top of aa_test.m describing usage. However, a few notes here might be helpful:
Our testing philosophy is that a larger collection of simpler testing scripts each focused on a specific analysis task is preferred to a smaller collection of elaborate scripts each exercising a vast range of functionality. Additionally, testscripts will only use open data. I tried to create a new testscript suite that reflects this philosophy as much as was practical (one exception is aatest_ds000114_fmri, which I tried to break into a preprocessing script and a modeling script, but the latter replicated the former).
Testscripts no longer attempt to download data. Automatic data download will always be problematic, especially when we don't control the hosting site. TBH, if you are a developer, you will probably keep test data around on your machine anyway. As such, I've structured the testscripts to use small datasets and reuse data as much as possible. The scripts currently lean heavily on ds000114 -- I don't have a feel for whether using a more diverse collection of data is critical or not.
For this first pass, I tied to reproduce the coverage of the scripts in the examples directory albeit omitting analyses that were redundant (to be clear: I haven't deleted anything in /examples -- I added new PR testscripts in /developer). There were some CBU scripts I left out because I could not find alternate (i.e., non-CBU) test data. For example, I was able to find an OpenNeuro dataset to test DTI (so I included a DTI testscript) but not ASL and MTI. Perhaps someone familiar with this functionality could eventually add new PR testscripts for it that uses OpenNeuro data.
Similarly, I wasn't comfortable updating the meeg test script (I've never done an meeg analysis in aa and eeglab refuses to run on my machine). As such, there is currently no new meeg testscript. Perhaps someone familiar with this functionality could eventually add a new PR testscript for it that uses OpenNeuro data.
Similarly, I didn't include "connected" pipeline testing in order to simplify aa_test. If this functionality is critical to users, we can include it eventually.
I withheld a few scripts that I think are either unnecessary or problematic. For example, I wrote a script for testing Freesurfer recon-all. Is that really worth the 4-6 hour runtime? We already test Freesurfer via defacing which runs faster. Also, I created a testscript for fieldmap analysis and identified a small OpenNeuro dataset that contains fieldmaps, but I couldn't get it past aa_processBIDS. It appears BIDS fieldmap input in aa is currently rather incomplete. As such, fieldmap testing will have to wait. Clearly, the testscript collection will evolve over time.
I provided the option to skip running a report (by leaving "reportname" blank in your parameter file). We don't use reporting because it almost always gives us trouble. There needs to be an effort to get it working properly under OS-X, but that is a project for another day.
The biggest unknown is whether the new code plays nice with qsub. I think it should work -- the core of aa_test isn't drastically different than the previous version -- but I can't run qsub here so I can't test it.
Other new stuff:
There's a template testscript in /developer documenting the new testscript layout. If you want to add a new testscript, this is the place to start.
I included a shell script testcoverage.sh in /developer that summarizes which modules don't yet appear in a testscript. Alas, we have much work to do before we attain significant test coverage...
I added aa_jpeg_crawler.m to /developer. This takes the aap struct from a completed or partial analysis and assembles all the jpeg images in the results directory tree into a surfable html file for a rapid visual check of analysis results using any browser.
Some changes to /examples:
I've added a few scripts to /examples that our students have found helpful. This includes a (very!) basic "helloworld" pipeline, a more useful helloworld extension that runs a model, and an implementation of the auditory tutorial from the SPM manual.
I added example_crawler.m that crawls the examples directory and lists the header information in each script to the command window. It's a fast way to see what examples are available (but requires cooperation from example script writers to include a helpful comment block at the top of the script).
I moved aamod_template_session.m from /examples to /developer (renamed: aamod_template.m). This file is more a "developer" thing and less an "example" thing.
Other changes/additions:
aamod_norm_write_epi.xml, which is a workaround for the domain wildcarding used in aamod_norm_write (domain wildcarding has never worked for us), an updated aas_runFSLcommand.m which fixes an OS-X environment bug, and aamod_reorienttomiddle_t2.xml which is needed by my testscripts.
I've updated README.md to reflect the new testing.
The intended design is to run aa_test with no options which runs all the scripts in developer/testscripts and writes a pass/fail log ("aa_test.log") to the current working directory. After checking the log, you can re-run a failed script by passing the name to aa_test and setting the debug flag. See the comment header in aa_test.m for further details.
M