Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix brainglobe atlas coordinates order #21

Closed
wants to merge 1 commit into from

Conversation

GuillaumeLeGoc
Copy link
Contributor

  • fix atlas aspect ratio
  • fix coordinates on mouse hover
  • fix typos

- fix coordinates on mouse hover
- fix typos
@GuillaumeLeGoc
Copy link
Contributor Author

Note that this works within ABBA GUI launched from python, but coordinates are still X-Z inverted when computing atlas coordinates in QuPath.

@NicoKiaru
Copy link
Member

Yep thanks, I still need to think a bit further this one

@NicoKiaru
Copy link
Member

Hello @GuillaumeLeGoc ,

There's the https://pypi.org/project/abba-python/#history prerelease 0.9.6.dev0. Could you please let me know when tyou have time if there is still this x / z issue, and if there's a simple way to reproduce it ?

(Sorry if I missed some other messages, I'm just writing a quick message so I won't forget)

@GuillaumeLeGoc
Copy link
Contributor Author

Hi @NicoKiaru, sorry for the delay, I was in holidays.
I've been trying the latest 0.9.6.dev0 pre-release from PyPI, while it installs and works great, the coordinates are still not corresponding to the regular ABBA Fiji.

In regular ABBA Fiji :

First coordinates is rostro-caudal (antero/posterior)
Second coordinates is dorso-ventral (up/down)
Third coordinates is medio-lateral (left/right)

In abba_python

First coordinates is medio-lateral (left/right)
Second coordinates is dorso-ventral (up/down)
Third coordinates is rostro-caudal (antero/posterior)

This is propagated in QuPath, when importing regions and adding the atlas coordinates to objects with the code snippet :

def pixelToAtlasTransform =
    AtlasTools
    .getAtlasToPixelTransform(getCurrentImageData())
    .inverse() // pixel to atlas = inverse of atlas to pixel

getDetectionObjects().forEach(detection -> {
    RealPoint atlasCoordinates = new RealPoint(3)
    MeasurementList ml = detection.getMeasurementList()
    atlasCoordinates.setPosition([detection.getROI().getCentroidX(), detection.getROI().getCentroidY(), 0] as double[])
    pixelToAtlasTransform.apply(atlasCoordinates, atlasCoordinates)
    ml.put('Atlas_X', atlasCoordinates.getDoublePosition(0) * 1000)
    ml.put('Atlas_Y', atlasCoordinates.getDoublePosition(1) * 1000)
    ml.put('Atlas_Z', atlasCoordinates.getDoublePosition(2) * 1000)
})

Atlas_X is Left/Right and Atlas_Z is rostro/caudal.
Also, brainglobe considers the regions appearing on the left on the screen belonging to the "left" hemisphere, and it is the other way around with regular ABBA.

@NicoKiaru
Copy link
Member

NicoKiaru commented Oct 30, 2024

Ok, so, after giving it some time with the help of @lacan:

1 - it's not abba_python which is relevant, it's the discrepancy between using an atlas from brainglobe and one from Java

2 - for the Allen Atlas taken from Java, everything is correct:
the coordinates, even though weird, are clear:
image

As you wrote:
x, first coordinate, is AP
y, second coordinate, is up/down (superior inferior)
z, third coordinate, is left/right

This is a global unique coordinate system. This coordinate system is independent of how the brain is sliced.

3 - for brainglobe, I think the key point is that they do not reason based on a single physical coordinate system. Rather they reason based on the indices of the voxel within a resliced stack. And the coordinate 0,0,0 may refer to different location based on how the atlas is sliced. The relevant pages are:

https://brainglobe.info/documentation/setting-up/image-definition.html

(And also brainglobe follows the z,y,x indexing convention (https://brainglobe.info/community/developers/conventions.html#axis-ordering-in-spatial-arrays).

0,0,0 means many different location depending on whether the atlas is sliced in asr, psl, spr, etc.

So what I can do is to reconstitute the coordinates so that they match the global Allen coordinates system. One annoying thing though, is that I need to apply this correction only for the Atlases compatible with the Allen CCFv3. So it adds an extra logic in abba-python.

I'll try to do it though.

@GuillaumeLeGoc
Copy link
Contributor Author

Thanks for the investigation !
Isn't there something to do with brainglobe-space to try and generalize instead of a per-atlas logic ?

@NicoKiaru
Copy link
Member

Indeed. But I don't see how to integrate this.

And the goal is to set the position of the slices in a unique global coordinate system. I don't want the user to specify anything when he has to do the export.

@NicoKiaru
Copy link
Member

This commit should fix the issue.

665ed5e

Let me know if/when you can test it

@GuillaumeLeGoc
Copy link
Contributor Author

I'm testing right now, but I got weird results, which I reproduced in the regular Fiji ABBA plugin...
I'm getting aberrant coordinates when using elastix spline transform (both 5.0.1 and 5.2.0). Regions look nice but the coordinates conversion fail (got negative, round coordinates).
I did not use ABBA since the last big update, and do not use much the spline transform so I'm not sure when it broke, nor if it is specific to my images... Transformation do look good though. I'll try and document a bit more in an issue in the main repo later on.

To the matter at hand :
Seems to work !
I tried :

  • Regular Fiji, Java CCFv3 (for reference)
  • abba_python, Java CCFv3
  • abba_python, brainglobe allen_mouse_10um

Exported regions back to QuPath, added some cells, computed centroids atlas coordinates -- got more or less the same results. Regions are now Left/Right-ed in a consistent manner, which is nice.

As expected, the previous behaviour is retained for other atlases, such as allen_cord_20um, as it is not DeepSlice-abled, so not CCFv3-compatible.
But, it would be very nice if it was possible to implement this for other atlases ? I see two options, instead of using DeepSlice to detect CCFv3 compatibility.

  • a new check box when importing images (or QuPath project) : "CCFv3-style space ?". If yes, then apply the transform.
  • another option, because I'm still a bit very confused with this whole space coordinates thing : why would it be any different for other atlases ? As long as they are in the same orientation (eg. antero-posterior, dorso-ventral, left-right), the same logic could be applied, eg. fix the space with the transform. The "orientation" properties of the BrainGlobeAtlas could be read, if it is "asr" or "asl", the transform can be applied.

For the latter option, of course, one could argue "why ABBA would mess-up the coordinates system of brainglobe atlases", to which I don't have an answer, except that it is what is done for Allen mouse brains already, to mimick the ABBA-Java behavior.
Anyway, food for thought ! And thank you very much for the commit.

Cheers,
Guillaume

@NicoKiaru
Copy link
Member

You're too fast! Thanks a lot! Give me a day or two to digest ;-)

@NicoKiaru
Copy link
Member

I'm testing right now, but I got weird results, which I reproduced in the regular Fiji ABBA plugin... I'm getting aberrant coordinates when using elastix spline transform (both 5.0.1 and 5.2.0). Regions look nice but the coordinates conversion fail (got negative, round coordinates). I did not use ABBA since the last big update, and do not use much the spline transform so I'm not sure when it broke, nor if it is specific to my images... Transformation do look good though. I'll try and document a bit more in an issue in the main repo later on.

I can't reproduce... Let's keep this aside for now.

For the rest ... do you really need to match coordinates from different atlases ? Also there's no need to match them with Java atlases since there's no Java equivalent.

I think I'll make the release in the current situation (integrating also the arbitrary slicing in the next Java version).

@GuillaumeLeGoc
Copy link
Contributor Author

My use case is immuno-staining of mouse slices from both the brain and the spinal cord.

I started to work on the brain with ABBA and QuPath, and made some downstream analysis scripts in QuPath to export the data and display it in Python, which uses the brain regions (QuPath annotations) with QuPath measurements (cells counts and the like) -- in which case, the space coordinates do not matter.
But I also use the cells atlas coordinates to compute and display spatial distributions in the three projections (rostro-caudal, medio-lateral and dorso-ventral). Furthermore, I use the medio-lateral coordinates to determine in which hemisphere a cell belong to.
Then came spinal cord images (around the time you released abba_python, which was great timing), which essentially behave the same as the brain : regions and atlas coordinates with "hemispheres" (eg. left/right).

Therefore, right now, the QuPath scripts need to be parametrized to specify "brain" (thus Java, thus fixed space) or "cord" (thus brainglobe, thus X/Z switch) to know if I need to flip the coordinates to make the downstream processes work as it does for the brain.

Again, this is perfectly acceptable as long as we're aware, but I'm always eager to make scripts where the users has to specify the fewest parameters as possible, and find it confusing that coordinates suddenly switch depending of their source.

Bottom line is, while I understand that it seems weird to modify atlases coordinates when there's no Java equivalent, in practice, more than half brainglobe atlases are mouse atlases (and even more if counting rodents in general), where one (at least, I) would expect consistent coordinates convention with respect to ABBA-Fiji, eg. first dimension (X) is rostro-caudal (antero-posterior), [...], whatever the source of the registration.

Anyways, thanks a lot for looking into this.

@NicoKiaru
Copy link
Member

Ok, I get it. I'll check what I can do.

@NicoKiaru
Copy link
Member

I exposed a bit more clearly the atlases that follow the convention in here:

atlases_using_allen_ccfv3_convention = [

I don't see an easy satisfactory way to deal with this right now. One option could be to specify which dimension corresponds to which axis in the exported transformation. But I see huge pain ahead. That'll be for next year.

@GuillaumeLeGoc
Copy link
Contributor Author

I feel you 😅
What about using the orientation property of the BrainGlobeAtlas instead of the hardcoded list ? If it's asr or asl it should be safe to assume that first (X) = A-P (rostro-caudal for mouse), second (Y) = superior-inferior (dorso-ventral for mouse), third (Z) = left-right, or am I still missing something more complex ?

@NicoKiaru
Copy link
Member

What about using the orientation property of the BrainGlobeAtlas instead of the hardcoded list ?

ahAH, thanks for telling me about this. At least this will give some standardisation for finding the coronal orientation.

What do you think of the following regarding dealing with orientation:

  • one has to specify who's X, Y and WZ (ap, if, rl)
  • one has to specify the index of x y and z, z can be indexed 0, y 1 and x 2, or x 0, y 1 z 2 or even crazy stuff y 0 x 2 z 1 if you really want to.

And I can put a few helper buttons for the commonly used cases.

One issue I see with that is that one needs to remember all these options when he re-opens a state. A state does not contain any information about the atlas used. I may need to change this because for the user to remember:

  • the atlas
  • the orientation for x y z (6x5x4 possibilities)
  • the index of x y z (6 possibilities)
    is a bit too much

@imagesc-bot
Copy link

This pull request has been mentioned on Image.sc Forum. There might be relevant details there:

https://forum.image.sc/t/fiji-friends-weekly-dev-update-thread/103718/11

@NicoKiaru
Copy link
Member

Finally I think I will choose something a bit more simple.

I will release a new version of the 2 Java atlases that will follow BrainGlobe convention. If you align everything with these, you'll be able to process all atlases with the same orientation convention. (though not CCFv3)

I will keep the previous atlases for backward compatibility and add new lines:

  • Rat (Brainglobe ASR convention)
  • Allen (Brainglobe ASR convention)

This has the big advantage of not changing the code at all.

@GuillaumeLeGoc
Copy link
Contributor Author

Hello,
Sorry for the delay, I was away and just came back.

Yes, I guess that could be a solution.
But I'm still confused about the conventions and don't get why Brainglobe ASR convention is not CCFv3.
Following this :
ccfv3
The origin is at most anterior, superior, left. First (x), second (y) and third (z) dimensions are, respectively, anterior-posterior, superior-inferior, left-right.
To me, this matches the "ASR" orientation used in brainglobe. What I might get wrong is that this is only true in coronal, and might change in sagittal, as the "The first dimension is the one that you are slicing" (ref) -- I never tried in sagittal.

In the end, I feel that it might be easier for me for users to specify what type of atlases was used (eg. Java or brainglobe) in the QuPath scripts (that switch X and Z depending on that), as we already registered quite a few brain atlases with the Java brains and I'd like to keep the CCFv3 convention.

@NicoKiaru
Copy link
Member

Ok, to the risk of adding to the confusion, here's how the atlas are stored by default in BrainGlobe:

image

Indeed if you take the underlying data (a tiff file I think) downloaded by brainglobe you will see this:

low z:

image

high z:

image

This is ASR because the top left corner at position 0,0,0 is anterio superior right (the left part of the image is the right part of the atlas).

Thus the first coordinate (x) is R to L, the second coordinate (y) is S to I; the second coordinate (z) is (A to P)


Now, if you follow the python convention and assume that the first coordinate is z, the second is y, and the third is x. This starts to ressemble CCFv3:

  • first dimension (x) z is R to L
  • second dimension y is S to I
  • third dimension (z) x is A to P

This ressembles the CCFv3 convention, except for the first axis which is flipped. In CCFv3, z=0 means that you are on the Left part of the brain (L to R), while here z=0 means that you are on the Right part of the brain.

And none of this is dependent on the slicing you choose. You keep the convention from the start, and you slice along x, y or z, but you don't rename any axis while performing the slicing.

@GuillaumeLeGoc
Copy link
Contributor Author

Thanks a lot for the extensive spatial course !

I read that CCFv3 is left/right symmetric by construction, so in those conditions I guess in term of naming the dimensions, brainglobe matches CCFv3 but the problem is more in the order, which is the actual problem since it's used in indexing.

Therefore, as you already understood and stated (I'm slow), there is no simple solution except changing convention altogether, which is problematic either way. Thus, I'm okay with how things are now and kinda make sense for my script to require to know what convention was used.

Repackaging Java atlases as you suggested could be an option but you may want to wait and see if there's an actual demand.

Many thanks !

@NicoKiaru
Copy link
Member

you may want to wait and see if there's an actual demand.

I think it's a good idea anyway - right now the allen and waxholm atlases do not match with the brainglobe atlases, both in terms of orientation and at the pixel level (the waxholm atlas has a pixel size of 39 micrometers in brainglobe, and 39.0625 for the Java version. With the new version they will match exactly at the pixel level the ones in BrainGlobe. And that will be useful.

@GuillaumeLeGoc
Copy link
Contributor Author

Yes, true, you're absolutely right, and I like the unifying vibe !
In the meantime, I guess we can close this PR that is out of date anyway.

@NicoKiaru
Copy link
Member

Hi @GuillaumeLeGoc ,

So the new version (0.9.11.dev0) contains the allen_mouse_10um_java atlas which should match the brainglobe orientation. There's also a windows installer:

https://github.com/BIOP/ijp-imagetoatlas/releases/tag/0.9.11.dev0

@GuillaumeLeGoc
Copy link
Contributor Author

Hey @NicoKiaru, I'm very sorry to get back to you only now.
I like the solution you implemented, that's very flexible. In my end I added options to support both conventions.
Thanks a lot !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants