Testing memeory management

[francopestilli]

I wonder whether the clear operation deleted by this pull request was duplicating memory inside the function.

Can we test if this works and is lighter on memory consumption?

@bacaron @DanNBullock

[DanNBullock]

If I'm not mistaken, a clear operation would reduce the memory footprint. The version that is proposed as a merge here deletes this clear operation which would increase the footprint. The version the wma segmentation app had when it was compiled was the one with the clear operation, so it should be the lighter weight version of the two.

What is the requested action with this pull request?

[francopestilli]

@bacaron has been noticing that this function doubles the memory allocation. I am suggesting this change to test whether the problem is in this line. The rationale below.

The line is simply creating an internal symlink to a variable.
allStreams=wbfg.fibers;

The variable was passed on into the function and exists outside the function. So the memory was allocated outside this function.
[superficialClassification] =bsc_segmentSuperficialFibers(wbfg, atlas);

If you clear the variable insdie the function clear wbfg then the clear command will require making a local copy of the memory allocated for wbfg inside the function. As a result, after the clear command two independent copies of the same data will need to exist one outside of this function wbfg and one inside of this function allStreamlines

Without the clear command a symlink will be used to create to variables pointing to the same memory allocation (unless a change is made inside the function to allStreamlines, I do not see this happened downstream).

Does it make sense?

[DanNBullock]

Intriguing, yes, that does make sense.

Theoretically then, should we just get rid of the variable all together, and just use wbfg.fibers? Typically I do that sort of thing for the sake of transparency, but if there's a risk of duplicating memory usage then maybe the slight bit of extra transparency isn't worth it? This may not be viable though, because, if I'm recalling this correctly, Matlab handles structures and particularly iterating over cell arrays within structures, not too gracefully. Something to test i guess @bacaron ?

Either way, I'd rather actually perform the test before making a change like this to the master branch, just to ensure that there's an improvement.

I talked with @bacaron about this a bit, and it seems that running a test of the (uncompiled) code locally using the matlab memory profiling tool (https://www.mathworks.com/help/matlab/performance-and-memory.html) would be advisable here. We may even be able to find other opportunities for memory improvements.

Edit:

Also, as a side note (and as another thing @bacaron noted) even if wma_tools does get ported over to python (like here) it seems that whatever memory issue we are having here will be even worse over there.

[francopestilli]

@DanNBullock sure as you guys prefer. This change will not affect the code functionality. hence, the direct pull to master.

[DanNBullock]

This change will not affect the code functionality. hence, the direct pull to master.

Good point. Also, thanks for the code inspection.

That being said, you've now made me wary/suspicious of my other uses of clear... Do you see any problems with these:
https://github.com/DanNBullock/wma_tools/search?q=clear ?

Edit:

Actually given how many of those there are, perhaps that's an over-demanding request. I'll try and look those over at some later point and see if there's any misuse going on.