pycortex icon indicating copy to clipboard operation
pycortex copied to clipboard

MNI coordinate calculation, storage, and display

Open smerdis opened this issue 10 years ago • 11 comments

This set of changes includes the anat_to_mni() function of the align module, as well as the code to include the calculated MNI coordinates in the CTM file, and JS stuff to load, display, and search this.

smerdis avatar Nov 03 '15 00:11 smerdis

This looks interesting; any chance you could post a screen shot of this so I can get a better idea how I might use it?

bcipolli avatar Nov 07 '15 15:11 bcipolli

image

This is a screenshot of the feature in use (latest version, ab216b6). The box in the top left appears when a vertex is selected, and the radio buttons toggle display of MNI and magnet iso-center coords. Clicking outside the brain hides the box again. Coordinates can be entered and searched in the box as well.

smerdis avatar Nov 11 '15 01:11 smerdis

Thank you! I will try this out in the next couple of weeks as I look more deeply into using PyCortex.

bcipolli avatar Nov 13 '15 04:11 bcipolli

Howdy,

I've been reviewing the code, and there are a few issues still to be addressed:

(1) The variable outfile is overwritten partway through mni_nl in surfinfo.py, which means that cortex.db.get_surfinfo(subject,type='mni_nl') does not save its output correctly. I fixed this by changing the intermediate outfile variable to tmpfile.

(2) In all other db.get_surfaceinfo calls, the variables that are saved are called left and right, which then are made into a cortex.dataset.views.Vertex object. The mni_nl() function gives the variables the names leftpts and rightpts. Also, the coordinates are transposed vs other types of surface info. These two things (variable names and coordinate orientation) prevented the npz file variables from being converted into a Vertex object.

Minor things: Technically the second set of coordinates (currently labeled "magnet") are not magnet coordinates at all - they are indices into the original functional volume. Thus they should be integers, not floats. Based on what displays now, I think using ceil would be the best way to convert them to ints.

Also - I just spoke with James, who said you guys have been having side conversations about some of these exact things. Please to post here if there are changes to the code! I will abandon my updates if there are already fixes in progress.

marklescroart avatar Nov 18 '15 18:11 marklescroart

Hey, good catch on the outfile being overwritten. I just fixed it in my code by changing the name of the supplied parameter to si_outfile. Is that an acceptable solution or do we want to enforce that first parameter being named outfile?

Also, which coordinates are transposed? This is probably related to the fact that our anatomicals are oriented differently than the standard, hence why I had to use fslreorient2std.

As a side note, do we really want the MNI coordinates to get transformed into Vertex objects?

smerdis avatar Nov 18 '15 19:11 smerdis

Mostly, we want consistency with other functions. So it makes more sense to me to change the internal variable name rather than the input variable name. And it does make more sense to me to have as consistent an output from the get_surfinfo function as possible. Some difference is necessary, because the MNI coordinates are 3 values per vertex instead of 1, but that seems to be automatically handled by converting them to a Vertex movie object (if the input is the right size). Also, to clarify, it isn't that x is transposed with z or anything, you just need to .T the left and right variables that are stored in the .npz file (not leftpts and rightpts) in order for them to be read into a Vertex object. This will also necessitate changing slightly the code in brainctm.py, but that should be fairly straightforward.

Also - I ran this on two subjects (MLfs2 and DSfs2), and got (in MLfs2, anyway) an error due to nans in the mni coordinates. It only appeared to be one vertex that was nan'd out, so I just wrapped the coordinates in np.nan_to_num - did you run into anything like that in MLfs2, Arjun?

marklescroart avatar Nov 18 '15 19:11 marklescroart

Okay, I re-patched the mni_nl() function to use tmpfile in the intermediate stages, and outfile at the end. Also fixed the naming and transpose issues, so the result should be a Vertex now.

I got a NaN or two with MLfs2, yeah. Probably a good idea to wrap in nan_to_num.

Still working on the mm-vox thing...

smerdis avatar Nov 18 '15 23:11 smerdis

Okay, I believe I've fixed the remaining issues. dfefa58 fixes the surfinfo filename and format, and c31c4e3 fixes the magnet-coords-in-vox-not-mm issue. the latter also fixes the bug where the coords of the search result vertex weren't pushed back to the coord box.

smerdis avatar Nov 19 '15 00:11 smerdis

Sorry to report that this still isn't quite ready to go. There were a few variable name changes that I had to fix in brainctm.py, and now that I have fixed them I'm still getting a CTM_INVALID_MESH error from openctm that I do not understand.

Arjun, I realize we're dragging this into your next rotation - can you possibly give me commit access to your repo? That way I can directly push the fixes that I've made there and just leave the pull request as it is.

marklescroart avatar Nov 24 '15 22:11 marklescroart

Hey, sorry for the delay, you should have commit access to my repo.

smerdis avatar Nov 30 '15 17:11 smerdis

I do - I've just been bogged down with other stuff. I did try to push some changes, but I fucked up the commit, and added it to another branch (smerdis-master instead of master), and got annoyed trying to un-fuck it.

=-P

M

On Mon, Nov 30, 2015 at 9:25 AM, Arjun Mukerji [email protected] wrote:

Hey, sorry for the delay, you should have commit access to my repo.

— Reply to this email directly or view it on GitHub https://github.com/gallantlab/pycortex/pull/135#issuecomment-160696015.

marklescroart avatar Nov 30 '15 17:11 marklescroart