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.

Sign up for GitHub

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

Fixes to Perens and Kim mouse atlases #271

Merged
merged 9 commits into from
May 9, 2024
Merged

Fixes to Perens and Kim mouse atlases #271

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
fc255e2
changing orientation of perens_lsfm
viktorpm Apr 22, 2024
499a8a7
adding missing arguments to create_region_mesh function
viktorpm Apr 23, 2024
92a1d68
adding missing arguments to create_region_mesh function
viktorpm Apr 23, 2024
a56766c
explanatory comments
viktorpm Apr 24, 2024
03192b1
replacing rotated_annotations with annotated volume
viktorpm Apr 30, 2024
68ffb4b
adding argument parser, allowing parallel processing
viktorpm Apr 30, 2024
3e91908
turn on parallel mesh extraction for perens
viktorpm May 1, 2024
dde0aa5
Update kim_mouse.py, turn off parallelization
viktorpm May 3, 2024
46390be
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] May 3, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 19 additions & 8 deletions brainglobe_atlasapi/atlas_generation/atlas_scripts/kim_mouse.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
__version__ = "1"

import argparse

Check warning on line 3 in brainglobe_atlasapi/atlas_generation/atlas_scripts/kim_mouse.py

View check run for this annotation

Codecov / codecov/patch

brainglobe_atlasapi/atlas_generation/atlas_scripts/kim_mouse.py#L3 

Added line #L3 was not covered by tests
import json
import multiprocessing as mp
import tarfile
Expand All @@ -23,7 +24,7 @@
from brainglobe_atlasapi.atlas_generation.wrapup import wrapup_atlas_from_data
from brainglobe_atlasapi.structure_tree_util import get_structures_tree

PARALLEL = False # disable parallel mesh extraction for easier debugging
PARALLEL = True # disable parallel mesh extraction for easier debugging

Check warning on line 27 in brainglobe_atlasapi/atlas_generation/atlas_scripts/kim_mouse.py

View check run for this annotation

Codecov / codecov/patch

brainglobe_atlasapi/atlas_generation/atlas_scripts/kim_mouse.py#L27 

Added line #L27 was not covered by tests


def create_atlas(working_dir, resolution):
Expand Down Expand Up @@ -117,9 +118,7 @@

tree = get_structures_tree(structures)

rotated_annotations = np.rot90(annotated_volume, axes=(0, 2))

labels = np.unique(rotated_annotations).astype(np.int32)
labels = np.unique(annotated_volume).astype(np.int32)

Check warning on line 121 in brainglobe_atlasapi/atlas_generation/atlas_scripts/kim_mouse.py

View check run for this annotation

Codecov / codecov/patch

brainglobe_atlasapi/atlas_generation/atlas_scripts/kim_mouse.py#L121 

Added line #L121 was not covered by tests
for key, node in tree.nodes.items():
if key in labels:
is_label = True
Expand All @@ -146,7 +145,7 @@
node,
tree,
labels,
rotated_annotations,
annotated_volume,
ROOT_ID,
closing_n_iters,
decimate_fraction,
Expand All @@ -170,7 +169,7 @@
node,
tree,
labels,
rotated_annotations,
annotated_volume,
ROOT_ID,
closing_n_iters,
decimate_fraction,
Expand Down Expand Up @@ -237,9 +236,21 @@


if __name__ == "__main__":
resolution = 10 # some resolution, in microns (10, 25, 50, 100)
# Create argument parser to pass resoulution as an argument
parser = argparse.ArgumentParser(

Check warning on line 240 in brainglobe_atlasapi/atlas_generation/atlas_scripts/kim_mouse.py

View check run for this annotation

Codecov / codecov/patch

brainglobe_atlasapi/atlas_generation/atlas_scripts/kim_mouse.py#L240 

Added line #L240 was not covered by tests
description="Create an atlas with a specified resolution."
)
parser.add_argument(

Check warning on line 243 in brainglobe_atlasapi/atlas_generation/atlas_scripts/kim_mouse.py

View check run for this annotation

Codecov / codecov/patch

brainglobe_atlasapi/atlas_generation/atlas_scripts/kim_mouse.py#L243 

Added line #L243 was not covered by tests
"--resolution",
type=int,
default=10,
help="Resolution in microns (10, 25, 50, 100)",
)
args = parser.parse_args()

Check warning on line 249 in brainglobe_atlasapi/atlas_generation/atlas_scripts/kim_mouse.py

View check run for this annotation

Codecov / codecov/patch

brainglobe_atlasapi/atlas_generation/atlas_scripts/kim_mouse.py#L249 

Added line #L249 was not covered by tests
Comment on lines +240 to +249
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are we adding an argument parser here?
I think it should be a loop over the resolutions we'd like to provide?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I think the atlas generation scripts are meant to (?) loop over resolutions.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It didn't do it. Only one atlas was generated when I submitted a job on the HPC and I didn't want to change the resolution value in the code manually, push the new version and submit a new job for each resolution. I might be missing something.

Copy link
Member

@alessandrofelder alessandrofelder May 1, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't want to change the resolution value in the code manually, push the new version and submit a new job for each resolution.

I agree this is a worthy goal - my point is that I think this should not be achieved by a command line argument, but rather with a for loop in the code (so the script generates all four atlases in the same run)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought this was already in the code, but it's what the main script (see #273) coordinates.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see what you mean now. Will I still have the option to easily specify which atlas I want to generate? For example, if I only need the 100um one, I wouldn't want to wait for the script to generate all the others.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think you will, no.
If you'd like that flexibility, you probably want to clone the repository on the HPC, and edit the script via the command line?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having thought about this, I think it's best to:

@alessandrofelder?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we commented at the same time @alessandrofelder. I think we ideally shouldn't add the parser to the script, but getting fixed versions of the atlases out should be a priority.


# Generated atlas path:
bg_root_dir = Path.home() / "brainglobe_workingdir" / "kim_mouse"
bg_root_dir.mkdir(exist_ok=True, parents=True)
create_atlas(bg_root_dir, resolution)

# Use the parsed resolution
create_atlas(bg_root_dir, args.resolution)

Check warning on line 256 in brainglobe_atlasapi/atlas_generation/atlas_scripts/kim_mouse.py

View check run for this annotation

Codecov / codecov/patch

brainglobe_atlasapi/atlas_generation/atlas_scripts/kim_mouse.py#L256 

Added line #L256 was not covered by tests
13 changes: 10 additions & 3 deletions brainglobe_atlasapi/atlas_generation/atlas_scripts/perens_lsfm_mouse.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
from brainglobe_atlasapi.atlas_generation.wrapup import wrapup_atlas_from_data
from brainglobe_atlasapi.structure_tree_util import get_structures_tree

PARALLEL = False # disable parallel mesh extraction for easier debugging
PARALLEL = True # disable parallel mesh extraction for easier debugging

Check warning on line 23 in brainglobe_atlasapi/atlas_generation/atlas_scripts/perens_lsfm_mouse.py

View check run for this annotation

Codecov / codecov/patch

brainglobe_atlasapi/atlas_generation/atlas_scripts/perens_lsfm_mouse.py#L23 

Added line #L23 was not covered by tests


### Additional functions #####################################################
Expand Down Expand Up @@ -145,7 +145,7 @@
SPECIES = "Mus musculus"
ATLAS_LINK = "https://github.com/Gubra-ApS/LSFM-mouse-brain-atlas"
CITATION = "Perens et al. 2021, https://doi.org/10.1007/s12021-020-09490-8"
ORIENTATION = "rai"
ORIENTATION = "asr"

Check warning on line 148 in brainglobe_atlasapi/atlas_generation/atlas_scripts/perens_lsfm_mouse.py

View check run for this annotation

Codecov / codecov/patch

brainglobe_atlasapi/atlas_generation/atlas_scripts/perens_lsfm_mouse.py#L148 

Added line #L148 was not covered by tests
ROOT_ID = 997
ATLAS_FILE_URL = "https://github.com/Gubra-ApS/LSFM-mouse-brain-atlas/archive/master.tar.gz"

Expand Down Expand Up @@ -248,7 +248,10 @@
node.data = Region(is_label)

# Mesh creation
closing_n_iters = 2
closing_n_iters = 2 # not used for this atlas
decimate_fraction = 0.2 # not used for this atlas

Check warning on line 252 in brainglobe_atlasapi/atlas_generation/atlas_scripts/perens_lsfm_mouse.py

View check run for this annotation

Codecov / codecov/patch

brainglobe_atlasapi/atlas_generation/atlas_scripts/perens_lsfm_mouse.py#L251-L252 

Added lines #L251 - L252 were not covered by tests

smooth = False

Check warning on line 254 in brainglobe_atlasapi/atlas_generation/atlas_scripts/perens_lsfm_mouse.py

View check run for this annotation

Codecov / codecov/patch

brainglobe_atlasapi/atlas_generation/atlas_scripts/perens_lsfm_mouse.py#L254 

Added line #L254 was not covered by tests
start = time.time()
if PARALLEL:
pool = mp.Pool(mp.cpu_count() - 2)
Expand All @@ -265,6 +268,8 @@
annotated_volume,
ROOT_ID,
closing_n_iters,
decimate_fraction,
smooth,
)
for node in tree.nodes.values()
],
Expand All @@ -287,6 +292,8 @@
annotated_volume,
ROOT_ID,
closing_n_iters,
decimate_fraction,
smooth,
)
)

Expand Down
Loading