BlueBrain / atlas-placement-hints

Tools to compute placement hints in the context of brain atlases.
Apache License 2.0
1 stars 2 forks source link

new regex with barrel cortex barrel names exception #14

Closed alTeska closed 2 months ago

alTeska commented 11 months ago

as discussed: https://github.com/BlueBrain/atlas-commons/pull/5

For layers 6a and 6b the regex still keeps the 6[ab] part, the rest is removed.

Uncertain why the original layer 3 regex had additional symbols: [^/]3, am I missing something?

alTeska commented 11 months ago

I tested this new regex with the following setup including barrel annotations:

HIERARCHY=/gpfs/bbp.cscs.ch/project/proj100/atlas/mouse/atlas-release-barrels-may2023/hierarchy.json
ANNOTATION=/gpfs/bbp.cscs.ch/project/proj100/atlas/mouse/atlas-release-barrels-may2023/brain_regions.nrrd
DVECTORS=/gpfs/bbp.cscs.ch/project/proj100/atlas/mouse/atlas-release-barrels-may2023/direction_vectors_isocortex_ccfv3.nrrd
OUTPUT=/gpfs/bbp.cscs.ch/project/proj100/atlas/mouse/placement-hints-test

atlas-placement-hints isocortex  --hierarchy-path $HIERARCHY  --annotation-path $ANNOTATION  --direction-vectors-path $DVECTORS --algorithm voxel-based --output-dir $OUTPUT

The newly created placement hints do not have the artefact mentioned in the issue https://github.com/BlueBrain/atlas-placement-hints/issues/12

Screenshot 2023-10-11 at 17 14 37

As discussed with @lecriste, we also checked if the regex is used anywhere else and it seems that direction-vector interpolation uses it here https://github.com/BlueBrain/atlas-direction-vectors/blob/d0b7331f2add43a9a3b185c9bc594f339daab01d/atlas_direction_vectors/app/direction_vectors.py#L212

and this is the metadata that needs to be updated to the regex proposed in this pull request: https://staging.nise.bbp.epfl.ch/nexus/bbp/atlas/resources/https:%2F%2Fbbp.epfl.ch%2Fneurosciencegraph%2Fdata%2F6a593672-9e7d-45aa-8740-19d79da9b3d3#JSON

lecriste commented 11 months ago

OK. Can I rerun the Atlas pipeline with this new version of isocortex_metadata.json?

alTeska commented 11 months ago

For me, yes!

mgeplf commented 9 months ago

Was the pipeline rerun w/ this @lecriste? If so, we can work on getting CI to pass, and get it merged.

lecriste commented 9 months ago

Not yet.

dkeller9 commented 3 months ago

@visood it looks like this issue is causing some problems downstream. Was the fix ever implemented/ run?

rai-pranav commented 3 months ago

Vishal and I , had looked at this issue, and what we found was that the issue with the earlier regexes could be potentially be resolved by setting "with_descendants": false within layers element in the isocortex_metadata.json . What is the rationale for "with_descendants": true in the current code?

lecriste commented 3 months ago

Vishal and I , had looked at this issue, and what we found was that the issue with the earlier regexes could be potentially be resolved by setting "with_descendants": false within layers element in the isocortex_metadata.json . What is the rationale for "with_descendants": true in the current code?

I don't know but are you sure that's enough to fix the issue @alTeska mentioned here?

lecriste commented 3 months ago

I tracked the file in the Atlas pipeline repo: https://bbpgitlab.epfl.ch/dke/apps/blue_brain_atlas_pipeline/-/blob/develop/metadata/isocortex_metadata.json Once you agree on the new version, please open an MR there as well and the pipeline will automatically consume the new file. @rai-pranav

rai-pranav commented 3 months ago

Right now, I guess you can update the current pipeline with what @alTeska regex solution. I will test separately , if my solution produces the same result or not.

lecriste commented 3 months ago

The placement hints produced with this version of isocortex_metadata.json and the new CCFv2022 BBPa annotation are here: /gpfs/bbp.cscs.ch/data/project/proj84/atlas_pipeline_runs/2024-05-26T20:43:55+02:00/placement_hints/

Let me know if they look good. @alTeska @rai-pranav

alTeska commented 2 months ago

I had a look at the new [PH]y.nrrd, there is no artefact in this placement hints, approved.

lecriste commented 2 months ago

Was the pipeline rerun w/ this @lecriste? If so, we can work on getting CI to pass, and get it merged.

@mgeplf, it seems we can proceed.

mgeplf commented 2 months ago

Ok, I also did a release: https://github.com/BlueBrain/atlas-placement-hints/releases/tag/v0.1.3