Closed aarongeller closed 1 year ago
Hi Aaron, thanks again for your contribution. Your changes are now incorporated in the main branch of the repository. Feel free to open another issue if you find another bug!
cool, thanks Raphael!
Hi @aarongeller, I am reopening this issue because it seems that your fix for the orientation issue you encountered caused a bug on our end: #10 explains the situation. I will be reverting back to our old splitbrain.m to avoid the issue, but I will also open a new branch to investigate what in your fix caused the issue. The hope is to find the issue and bring back your version once we can fix the bug.
To kickstart this process, could you provide a bit more detail on the issue that prompted you to write this fix? Does it only happen when you use views other than the 'c' view? We do not use anything else than coronal views and are thinking of removing the orientation parameter altogether in a future version. However, if this is something that you use, I am happy to work toward finding a solution so that we retain the functionality.
The branch I will use to work on retaining your fix is multiple-orientations-splitbrain-test.
Hi Raphael-
The issue with brain flipping was occurring with any orientation, including 'c'. It was caused because the mesh partition provided by splitFV was not in the expected order. It was surmountable by adjusting the variable "thegap" and I believe that is was the original coder was referring to with the comment in the code regarding that variable. Obviously if my version is causing issues for you reverting is reasonable.
Yes I understand your issue. I am new to the codebase so sometimes it takes me some time to understand the code completely, I hope you'll forgive me. I have reverted to our old version and it seems to have fixed the issue for now, although I would like to investigate and find a way to integrate your changes.
I'll be looking into it over the next week/longer if necessary. Like I mentioned previously, the aim is to bring back your version in the future.
Sorry if this causes you trouble, I hope we can fix this quickly. Thanks again for all your contributions!
Can you send me a dataset which exhibits issue 10? If I get a chance I can take a look at how it runs on my end.
I'll look into that!
Linking #15 for documentation, this issue becomes a priority.
Found the bug! We used the wrong electrodefile for patients with high density grids. Will fix the patients and merge this fix again.
cool!
I hope to be able to fix #18 before bringing this back. It does not seem as though this would cause an issue in new places (except for one particular patient on our end), but I still hope to figure everything out before merging.
Hi Aaron, just to clarify the issue you were facing: you were encountering some brains that were flipped like in the following image?
If I understand correctly, your fix chooses the opposite side of the mesh in those cases to make sure that the slice faces the viewer. The current problem, and the only thing that stops me from bringing back your changes, is that on our end it seems that the side of the brain that is facing the viewer is not made transparent. Therefore, it hides from view the side of the brain we are interested about. Here is an example of this:
I am wondering if you made any other changes to the codebase, for this fix or maybe even before. I am especially curious about changes to the OPSCEAsurfslice.m file.
Once again, thanks a lot for all your contributions. Hopefully we can resolve this soon.
Hi Raphael-
Actually, I think my use of "flip" was different. It appears that the brain for ID in the top figure is "flipped" in the sense that it's not oriented optimally for the viewer. What I was seeing pertained just to slicing as done by OPSCEAsurfslice for depth electrodes, and involved display of the wrong half of the brain. An example for your plot would be that the anterior half of the brain would be shown for AD instead of the posterior one, and therefore the depth electrode would be blocked from view.
Now, if the issue you're referring to is how the PID electrode is displayed in the bottom figure is splitting the brain with residual left hemisipheric mesh, that's something I handle explicitly by defining a sagittal view ('s' mode) in splitbrain.
I have made several modifications both to OPSCEA.m and OPSCEAsurfslice.m and you're welcome to have my versions, if you'd like. My version of OPSCEA does not use the sagittal view, but I forked my own version of OPSCEA just for structural visualization that does. You're welcome to any of this code.
Best,
Aaron
Hi Aaron, update on this issue: I was able to find a fix! I moved your orientation_good()
function to OPSCEAsurfslice()
and used it to rotate the brain in the cases where needed. The current version fixes the issues I have encountered on my end, and I believe that it would fix yours as well, although I cannot test. If something is not addressed, feel free to open an issue and we will work to fix any problem that comes up. Once again, thanks for your help and patience!
Hi Raphael-
Actually, I think my use of "flip" was different. It appears that the brain for ID in the top figure is "flipped" in the sense that it's not oriented optimally for the viewer. What I was seeing pertained just to slicing as done by OPSCEAsurfslice for depth electrodes, and involved display of the wrong half of the brain. An example for your plot would be that the anterior half of the brain would be shown for AD instead of the posterior one, and therefore the depth electrode would be blocked from view.
Now, if the issue you're referring to is how the PID electrode is displayed in the bottom figure is splitting the brain with residual left hemisipheric mesh, that's something I handle explicitly by defining a sagittal view ('s' mode) in splitbrain.
I have made several modifications both to OPSCEA.m and OPSCEAsurfslice.m and you're welcome to have my versions, if you'd like. My version of OPSCEA does not use the sagittal view, but I forked my own version of OPSCEA just for structural visualization that does. You're welcome to any of this code.
Best,
Aaron
I don't think that that will be necessary for now, but thanks very much for the offer! I will keep it in mind. I have been thinking that one day it would be good to merge everything we both have into one cohesive codebase that we both us. If that would be of interest to you we can discuss that in the future. Thanks again for contributing!
sounds good, happy to contribute.
splitbrain.m often chooses the wrong mesh set, causing flipped brains and requiring manipulation of the variable
thegap
to force the right choice. I added a functionorientation_good
that solves this. My version of splitbrain supports slicing for coronal, axial, sagittal and "oblique coronal" views and is pasted below.