Skip to content
This repository was archived by the owner on Feb 26, 2025. It is now read-only.

Conversation

alTeska
Copy link
Member

@alTeska alTeska commented Oct 4, 2023

as discussed: BlueBrain/atlas-commons#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
Copy link
Member Author

alTeska commented Oct 11, 2023

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 #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
Copy link

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

@alTeska
Copy link
Member Author

alTeska commented Oct 11, 2023

For me, yes!

@mgeplf
Copy link
Collaborator

mgeplf commented Nov 20, 2023

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

@lecriste
Copy link

Not yet.

@dkeller9
Copy link

dkeller9 commented May 15, 2024

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

@rai-pranav
Copy link

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
Copy link

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
Copy link

lecriste commented May 24, 2024

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
Copy link

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
Copy link

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
Copy link
Member Author

alTeska commented Jun 12, 2024

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

@lecriste
Copy link

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
mgeplf previously approved these changes Jun 12, 2024
@mgeplf mgeplf self-requested a review June 12, 2024 12:11
@mgeplf mgeplf dismissed their stale review June 12, 2024 12:12

tests aren't passing

@mgeplf mgeplf merged commit 1a9e689 into main Jun 12, 2024
@mgeplf mgeplf deleted the barrel-regex branch June 12, 2024 12:13
@mgeplf
Copy link
Collaborator

mgeplf commented Jun 12, 2024

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.

5 participants