brainlife / app-AFQclean

(deprecated by Remove Tract Outliers App) This service cleans the output from AFQ and WMA using AFQ's AFQ_removeFiberOutliers function. For more information on the inputs of this application, please read the documentation at the top of the function: https://github.com/yeatmanlab/AFQ/blob/master/functions/AFQ_removeFiberOutliers.m
0 stars 2 forks source link

'classification' not updated #4

Open giulia-berto opened 5 years ago

giulia-berto commented 5 years ago

When saving the output.mat file, fg_classified is updated but classification is not. fg_classified = fg_classified_clean; save('output.mat', 'fg_classified', 'classification'); Don't you think it would be better to update also classification?

soichih commented 5 years ago

I actually don't see where classification is defined in this function. @kitchell Can you explain?

kitchell commented 5 years ago

its not defined. It is part of the fg classified that is read in from AFQ and saved down again.

If someone wants to add that, feel free. It hasn't caused problems thus far.

giulia-berto commented 5 years ago

Exactly. It is loaded with this: load(config.afq_fg); which loads the classification of the source AFQ (not cleaned). Then it is saved like it is. I was relying on the fact that also classification was updated.

kitchell commented 5 years ago

Ah, we typically use fg classified moving forward. The built in afq cleaning function, which we are using here, only updates fg classified. I wont be able to add any changes for the classification structure any time soon. If you are able to figure out a solution, feel free to make a pull request, otherwise Dan might be able to @DanNBullock

soichih commented 5 years ago

@DanNBullock Mohammad just pointed out that this is still an issue. Should we just remove "classification" from the save()?

DanNBullock commented 5 years ago

That would not be a classification or wma output at that point. https://brainlife.io/app/5c59dc65464fde003639ebb2 this app does this action appropriately. I'm not sure if it's outputting fg_classified vs tracts. Should be an easy fix if it isn't doing fg_classified.

Dan

On Mon, Mar 4, 2019 at 4:00 PM Soichi Hayashi notifications@github.com wrote:

@DanNBullock https://github.com/DanNBullock Mohammad just pointed out that this is still an issue. Should we just remove "classification" from the save()?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/brain-life/app-AFQclean/issues/4#issuecomment-469419872, or mute the thread https://github.com/notifications/unsubscribe-auth/AMt2vbNZ9t5dOiCAu-hwbxIfAJyuI2Wgks5vTYmDgaJpZM4YcLvZ .

soichih commented 5 years ago

I am not sure what we need to do with classification.. but just to keep this App working with wmc data without classification, I've updated the save() with the following.

if exist('classification','var')
    save('output.mat', 'fg_classified', 'classification');
else
    save('output.mat', 'fg_classified');
end

We need to discuss what we should do with classification.

soichih commented 5 years ago

I've discussed with Giulia and we agreed that we must do following to this App.

1) The new neuro/wma datatype no longer outputs fg_classified, so we must drop it from the output. This means we must update the classification structure to have cleaned fibers set to 0 before saving it as output, otherwise the output from AFQ_removeFiberOutliers is lost.

HINT: AFQ_removeFiberOutliers actually outputs fb and keep. keep contains information about which fibers are kept and which are not. We can apply this information to the input classification array to produce the "cleaned" version of classificaiton.

2) If input dataset has /surfaces, it should bypass it by creating a symlink.

3) (optionally) I believe "keep" from the AFQ_removeFiberOutliers could be saved down to the raw output so that user has a way to storing which fibers are cleaned out and which are not. This and input wma can be used to create a cleaned version of input track.tck, for example.

I would volunteer for these changes unless someone else wants to take it.

soichih commented 5 years ago

@DanNBullock pointed out that app-AFQclean should be deprecated by his new Remove Tract Outliers App. I've updated the description for this repo to reflect that

(deprecated by Remove Tract Outliers App)