{"action":"created","issue":{"url":"https://api.github.com/repos/MRtrix3/mrtrix3/issues/422","labels_url":"https://api.github.com/repos/MRtrix3/mrtrix3/issues/422/labels{/name}","comments_url":"https://api.github.com/repos/MRtrix3/mrtrix3/issues/422/comments","events_url":"https://api.github.com/repos/MRtrix3/mrtrix3/issues/422/events","html_url":"https://github.com/MRtrix3/mrtrix3/issues/422","id":122171035,"number":422,"title":"(ms)dwi2response & sh2response: Resolving naming before release","user":{"login":"Lestropie","id":5637955,"avatar_url":"https://avatars.githubusercontent.com/u/5637955?v=3","gravatar_id":"","url":"https://api.github.com/users/Lestropie","html_url":"https://github.com/Lestropie","followers_url":"https://api.github.com/users/Lestropie/followers","following_url":"https://api.github.com/users/Lestropie/following{/other_user}","gists_url":"https://api.github.com/users/Lestropie/gists{/gist_id}","starred_url":"https://api.github.com/users/Lestropie/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Lestropie/subscriptions","organizations_url":"https://api.github.com/users/Lestropie/orgs","repos_url":"https://api.github.com/users/Lestropie/repos","events_url":"https://api.github.com/users/Lestropie/events{/privacy}","received_events_url":"https://api.github.com/users/Lestropie/received_events","type":"User","site_admin":false},"labels":[{"url":"https://api.github.com/repos/MRtrix3/mrtrix3/labels/help%20wanted","name":"help wanted","color":"159818"},{"url":"https://api.github.com/repos/MRtrix3/mrtrix3/labels/question","name":"question","color":"cc317c"}],"state":"open","locked":false,"assignee":null,"milestone":{"url":"https://api.github.com/repos/MRtrix3/mrtrix3/milestones/1","html_url":"https://github.com/MRtrix3/mrtrix3/milestones/MRtrix3%203.0%20release%20","labels_url":"https://api.github.com/repos/MRtrix3/mrtrix3/milestones/1/labels","id":1036648,"number":1,"title":"MRtrix3 3.0 release ","description":"","creator":{"login":"jdtournier","id":3221000,"avatar_url":"https://avatars.githubusercontent.com/u/3221000?v=3","gravatar_id":"","url":"https://api.github.com/users/jdtournier","html_url":"https://github.com/jdtournier","followers_url":"https://api.github.com/users/jdtournier/followers","following_url":"https://api.github.com/users/jdtournier/following{/other_user}","gists_url":"https://api.github.com/users/jdtournier/gists{/gist_id}","starred_url":"https://api.github.com/users/jdtournier/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jdtournier/subscriptions","organizations_url":"https://api.github.com/users/jdtournier/orgs","repos_url":"https://api.github.com/users/jdtournier/repos","events_url":"https://api.github.com/users/jdtournier/events{/privacy}","received_events_url":"https://api.github.com/users/jdtournier/received_events","type":"User","site_admin":false},"open_issues":35,"closed_issues":66,"state":"open","created_at":"2015-03-24T16:09:24Z","updated_at":"2015-12-16T12:07:48Z","due_on":"2016-01-01T00:00:00Z","closed_at":null},"comments":8,"created_at":"2015-12-15T01:26:57Z","updated_at":"2015-12-17T15:09:13Z","closed_at":null,"body":"We need to decide what to do about these before we release. Supersedes #105.\r\n\r\nHere's my current thinking, feel free to modify or suggest complete alternatives:\r\n\r\n* Keep `dwi2response`, but rename it to `dwi2sf`, and output the single-fibre voxel mask by default; only give the final response function if explicitly requested by the user.\r\n\r\n* Rename `sh2response` to `sf2response`, and receive as primary input a DWI rather than SH; this will additionally allow for selecting the shell(s) for which response(s) should be estimated (which should simplify `msdwi2response`).\r\n\r\nHopefully that makes it clear that there are in fact two steps: single-fibre voxel selection, and response function estimation based on those voxels. Just using the (optional) response function output from `dwi2sf` is a shortcut that will probably satisfy most users, but hopefully the fact that the SF voxel mask is the default output will make people question that and look into alternative ways to get the SF mask rather than just reporting that they can't get a response function.\r\n\r\nHopefully it also separates `msdwi2response` from what is currently `dwi2response`, as their operations are actually quite different currently."},"comment":{"url":"https://api.github.com/repos/MRtrix3/mrtrix3/issues/comments/165478861","html_url":"https://github.com/MRtrix3/mrtrix3/issues/422#issuecomment-165478861","issue_url":"https://api.github.com/repos/MRtrix3/mrtrix3/issues/422","id":165478861,"user":{"login":"jdtournier","id":3221000,"avatar_url":"https://avatars.githubusercontent.com/u/3221000?v=3","gravatar_id":"","url":"https://api.github.com/users/jdtournier","html_url":"https://github.com/jdtournier","followers_url":"https://api.github.com/users/jdtournier/followers","following_url":"https://api.github.com/users/jdtournier/following{/other_user}","gists_url":"https://api.github.com/users/jdtournier/gists{/gist_id}","starred_url":"https://api.github.com/users/jdtournier/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jdtournier/subscriptions","organizations_url":"https://api.github.com/users/jdtournier/orgs","repos_url":"https://api.github.com/users/jdtournier/repos","events_url":"https://api.github.com/users/jdtournier/events{/privacy}","received_events_url":"https://api.github.com/users/jdtournier/received_events","type":"User","site_admin":false},"created_at":"2015-12-17T15:09:13Z","updated_at":"2015-12-17T15:09:13Z","body":"Glad there's some form of consensus emerging! Quick comments on specific points:\r\n\r\n##### Replacing `amp2sh | sh2response` with a direct `sf2response`\r\n\r\nFine by me! the whole point is to allow these kinds of modifications to be made trivially, and each stage to be tested and made available independently. I was also thinking that the `amp2sh` call might be unstable on ill-conditioned data, and that we could do a lot better by doing the DWI->response fit in one go. So absolutely, if you want to have a go, I'd certainly be happy to have these calls replaced with something better. Or to have whatever replaces it also allow operation on the SH coefficients directly.\r\n\r\nI'd also be happy to replace the `sh2peaks` call with something fixel-based (as you suggest in #426) if we find that it consistently performs better (and faster).\r\n\r\n##### Renaming `sh2response` to `mask2response`\r\n\r\nI don't like the name much (for the reasons @dchristiaens already mentioned above), but if it really gives you the irrits... I'm not sure why you'd need to change it though, it's not conflicting with any other command you might be planning to replace it with, is it...? Besides, it really has to be SH, even for lmax=0... Otherwise the scaling will be off. But as you suggest, that wouldn't matter much if this was replaced with a `sf2response` command that allowed operation on either raw DWI or SH coefficients. Or maybe `responsegen`...?\r\n\r\n\r\n##### Starting with a sharp response\r\n\r\n> Check your data.\r\n\r\nJust did. You're right, it's not all that clean with a sharp response if you go for the full lmax=8 fit - see images below. Much better if restricted to lmax=4 (next image). Nonetheless, using a fat lmax=2 response seems a very bad idea to me (last image) - it can't actually detect crossing fibres at all (you'd need at least lmax=4 for that)... \r\n\r\n![screenshot0000](https://cloud.githubusercontent.com/assets/3221000/11872867/8febf2d6-a4cf-11e5-9e78-60b638015779.png)\r\n![screenshot0002](https://cloud.githubusercontent.com/assets/3221000/11872871/92cb1428-a4cf-11e5-9fd0-dcdfc3c5fdbe.png)\r\n![screenshot0001](https://cloud.githubusercontent.com/assets/3221000/11872870/913aa51a-a4cf-11e5-9bc2-688e6ace0306.png)\r\n\r\n\r\n##### Supplying another black box\r\n\r\nI thought the whole discussion around scripting was to make sure this wasn't a black box?!? I admit I might have given the wrong impression by creating a pull request for it - I have no intention for this to be merged at this stage, it's for too rough round the edges. I just thought I'd push to a separate branch so people can test it on their own data before we take this further. Of course we'll need to test it and modify extensively to make it clean and provide access to all the options, etc. before including it in the main release...\r\n\r\nAnd as I've mentioned before, I've tested it on other data sets too, where things are much more challenging (neonatal b=500, 47 DW directions, very little angular contrast). But I'd like people to try it on ex vivo or muscle data too, which would be equally challenging. There's a few issues there, I'm refining the approach, and I'll commit an update on the `estimate_response` branch in a bit.\r\n\r\nAlso, on the data I was initially using for testing (the old b=3000, 150 DW directions BRI data set), `dwi2response` failed after a 10 minute runtime with the 'no voxels left in mask' error. I think there's a good argument to be made for ensuring that whatever strategy we do use works out of the box on _any_ dataset acquired with the settings we ourselves recommend... So if we're going to put disclaimers and stringent tests on any alternative approaches, they should equally apply to the existing approach. Regardless, I completely agree that whatever replacement we come up with will need to be extensively tested and polished before we push it out. \r\n\r\n##### Thresholding on number of voxels\r\n\r\nYou mention you had data where this wasn't appropriate - what was the issue there? I can't think of a situation where it wouldn't be the right thing to do... We want the sharpest response supported by the data - not just the average over those voxels that _appear_ single-fibre. I plucked the 300 number out of the air, but the rationale was to avoid being overly influenced by voxels that were artificially sharper due to noise (hence a largish number), but still reflected the sharpest subset of the data. I also note that `dwi2response` often ends up with way more voxels included, but I'm not convinced that's necessarily a good thing in terms of ensuring that the voxels included genuinely have the least dispersion, and hence the best estimate of the intrinsic response. \r\n\r\nIn any event, that is again something that can trivially be changed if needed - it's just one line in the script. I've had one data set that produced a funny-looking response, and inspecting the SF mask quickly showed the problem: there was a prominent fat artefact, and many of the voxels ended up in there. Easy enough fix (erode the mask...), but I'm not sure there's a great deal we can do about this when the data are artefacted in such a way as to exhibit high anisotropy within the initial brain mask.\r\n\r\n##### Not using ArgParse\r\n\r\nDude, I had no time to figure it out! I just wanted to cobble a quick proof of concept together quickly, expecting a proper version to be scripted in Python later. But even then, I don't think it's a problem to use bash scripts if that's what people are comfortable with - it may not be as powerful as Python, but you can go a surprisingly long way with it... What's missing is the whole argument parsing interface, which is why it will need to be ported to Python eventually: too many options for each stage...\r\n\r\nAnyway, more on #426..."}}