Skip to content
This repository has been archived by the owner on Feb 23, 2024. It is now read-only.

Add ADMBA 3D developing mouse atlases #25

Merged
merged 3 commits into from
Jul 1, 2022

Conversation

yoda-vid
Copy link
Contributor

Description

What is this PR

  • Bug fix
  • Addition of a new feature
  • Other

Why is this PR needed?

This PR extends the mouse_e15_5 script by @pr4deepr in #11 to build all eight atlases in the 3D reconstructed version of the Allen Developing Mouse Brain Atlas series. Thanks for hosting these atlases!

What does this PR do?

This build script is essentially the same as the original version except for the addition of a data class to configure the atlas parameters, which is implemented for each atlas in the same script. I've kept @pr4deepr's name as the package author as he built the original script.

Am I correct in understanding that the atlas generator reorients the atlas from the given orientation to ASR orientation? I input the orientation as PIR while the output metadata says ASR. Original is on the left, output on the right (or is it PSR?):

image

Also, are the resolutions in x, y, z order, and are they unchanged during atlas generation? These atlases are slightly anisotropic (same resolutions as the original), so I wanted to double-check and have listed the resolutions in this order.

References

#11, which introduced the atlas creation script for the E15.5 atlas as well as a GUI. This current PR focuses on the atlas build script while the GUI implementation can continue in the original PR.

How has this PR been tested?

Built and tested locally (but haven't tested the meshes). I have run into sporadic connection errors during building at times, presumably network errors with the host (eg right at the moment).

Is this a breaking change?

No

Does this PR require an update to the documentation?

No, but it could be useful to update the docs on the orientation and also on the resolution order for non-isotropic atlases.

Checklist:

  • The code has been tested locally
  • Tests have been added to cover all new functionality (unit & integration)
  • The documentation has been updated to reflect any changes
  • The code has been formatted with pre-commit

Extend the `mouse_e15_5` script by @pr4deepr to build all the atlases in the 3D reconstructed version of the Allen Developing Mouse Brain Atlas series. This build script is essentially the same except for the addition of a data class to configure the atlas parameters, which is implemented for each atlas.
@adamltyson
Copy link
Member

Hey @yoda-vid, thanks for raising this PR. I'm looking through it now, but to answer your questions:

Am I correct in understanding that the atlas generator reorients the atlas from the given orientation to ASR orientation? I input the orientation as PIR while the output metadata says ASR. Original is on the left, output on the right (or is it PSR?):

You're correct, but the output doesn't look right. Is the orientation of all the atlases the same? I took a look at the P56 one, and loading in both imageJ/napari it looks like it's lsa (or rsa, can't tell as it's symmetrical).

Also, are the resolutions in x, y, z order, and are they unchanged during atlas generation? These atlases are slightly anisotropic (same resolutions as the original), so I wanted to double-check and have listed the resolutions in this order.

The resolutions here (and in all brainglobe software) aren't x, y, z. They're in the same order as the orientation of the data. Based on the above, these may be incorrect.

No, but it could be useful to update the docs on the orientation and also on the resolution order for non-isotropic atlases.

I think we need a lot more docs around this generally!

@adamltyson
Copy link
Member

As far as I can tell, these atlases all look good. It's just the resolution and orientation that needs checking.

Up to you, but I think both you and @pr4deepr should share the packaging credit.

@pr4deepr
Copy link

Up to you, but I think both you and @pr4deepr should share the packaging credit.

Agreed.

@yoda-vid
Copy link
Contributor Author

Thanks for your review, @adamltyson!

You're correct, but the output doesn't look right. Is the orientation of all the atlases the same? I took a look at the P56 one, and loading in both imageJ/napari it looks like it's lsa (or rsa, can't tell as it's symmetrical).

Yes, all of these atlases are in the same orientation. In the screenshot above, would you consider the image on the left (the original input) to be PIR? I'm admittedly not super familiar with the orientation nomenclature and just got PIR from the Allen docs for the original atlases. And the output image on the left is PSR? I could very well be wrong here so feel free to correct.

The resolutions here (and in all brainglobe software) aren't x, y, z. They're in the same order as the orientation of the data. Based on the above, these may be incorrect.

Ok makes sense, so the resolutions should be in the order of the output data, correct? For ASR orientation, these atlases would be in n, n, m resolution (anisotropic along the sagittal planes). I can fix it once I've figured out the right orientation.

As far as I can tell, these atlases all look good. It's just the resolution and orientation that needs checking.

Sure, happy to share with @pr4deepr ... it's definitely been fun working on this together!

@adamltyson
Copy link
Member

Yes, all of these atlases are in the same orientation. In the screenshot above, would you consider the image on the left (the original input) to be PIR? I'm admittedly not super familiar with the orientation nomenclature and just got PIR from the Allen docs for the original atlases. And the output image on the left is PSR? I could very well be wrong here so feel free to correct.

Just to confuse things, we use a different coordinate system to the Allen. We use the Python/Numpy system where the origin is top left corner of the first plane of the image. The first axis is then front to back, the second top to bottom, and the third left to right. With the caveat that one can't tell left from right just by eye, the left looks like lsa (or rsa) and the right is lia (or ria). The other caveat is that my developmental neuroanatomy may not be very good anymore!

If it helps, if the first plane of this image was olfactory bulb, it would be asr (or asl). If the first plane was cerebellum, it would be psr or psl.

allen_mouse

If the first plane of this image was the top of the brain it would be sal/sar. If the first plane was the base it would be ial/iar.
perens_lsfm

Ok makes sense, so the resolutions should be in the order of the output data, correct? For ASR orientation, these atlases would be in n, n, m resolution (anisotropic along the sagittal planes). I can fix it once I've figured out the right orientation.

Yes, this should be in the order of the output files, as they're read when the atlas is used.

Change the input orientation to LSA to fit the NumPy convention used in BrainGlobe. Add docstrings describing the orientation and resolution assumptions in the configuration data class.
@yoda-vid
Copy link
Contributor Author

Just to confuse things, we use a different coordinate system to the Allen. We use the Python/Numpy system where the origin is top left corner of the first plane of the image.

This is super helpful, thanks @adamltyson! Makes much more sense now. I also take it that the directions point "from" as opposed to "to" ? I think your explanation/examples here would be great for the docs! I've added them into our atlas script as well.

I changed the input orientation to lsa and get asr/asl as the output now. Looks like the resolution matches the orientation now as-is. Here is the output for the P4 atlas (original on left, output on right, without resolution scaling):

image

yoda-vid added a commit to sanderslab/magellanmapper that referenced this pull request Jun 23, 2022
Per discussion at brainglobe/bg-atlasgen#25 (comment), the BrainGlobe resolutions correspond to the image's orientation in NumPy space. Commit 454fa60 introduced resolution reversal, but the resolution should be used as-is. Manually revert this change.
@adamltyson
Copy link
Member

This looks great to me! Any thoughts @vigji or @FedeClaudi before I merge and upload these? The meshes could do with smoothing, but that's a general issue, not specific to these.

Thanks @yoda-vid & @pr4deepr!

@adamltyson
Copy link
Member

Going to merge, as all looks good to me.

@adamltyson adamltyson merged commit 6e2f8c7 into brainglobe:master Jul 1, 2022
@yoda-vid yoda-vid deleted the admba_3d_mouse branch July 4, 2022 07:06
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants