BlueBrain / atlas-densities

Tools to compute densities in the context of brain atlases.
Apache License 2.0
2 stars 7 forks source link

Hard-coded CCFv2 slice numbers for gene markers should be removed and set as an input parameter #66

Open Sebastien-PILUSO opened 5 months ago

Sebastien-PILUSO commented 5 months ago

The fit_average_densities command takes an indirect input which is realigned_slices_ccfv2.json via fit_average_densities_ccfv2_config.yaml (see https://github.com/BlueBrain/atlas-densities/tree/main/atlas_densities/app/data/markers).

However, this file (realigned_slices_ccfv2.json) embeds hard-coded slice coordinates in CCFv2 for manually chosen experiment IDs among the Allen ISH data portal for each used marker.

First, this prevents any genericity in the input gene markers one user can give to the pipeline, where one must use these exact same data if we want the pipeline to work properly. Second, one atlas-densities user does not know such coordinates are used as it is hidden in fit_average_densities_ccfv2_config.yaml and no documentation is provided in the main README.

This is at the junction between the deep-atlas-pipeline and the atlas-densities pipeline. Actually, in addition to providing the realigned slices, the deep-atlas-pipeline is producing an attached JSON file with the slice coordinates of whatever selected gene ID you want.

What I suggest:

  1. create a function which converts all marker's attached JSON files into thisrealigned_slices_ccfv2.json and add it at the beginning of the fit_average_densities process,
  2. set this realigned_slices_ccfv2.json as a direct input file to give to the fit_average_densities command,
  3. supplement the main documentation to inform the user of the content of this required file.
mgeplf commented 5 months ago

the deep-atlas-pipeline is producing an attached JSON file with the slice coordinates of whatever selected gene ID you want.

Can you point me to such a file?

and no documentation is provided in the main README.

The main README is quite large, but there is this: https://github.com/BlueBrain/atlas-densities/tree/main/atlas_densities/app/data/markers Which, I hope, covers the main points.

I think it would be good to move away from needing the fit_average_densities_ccfv2_config.json file, as all the same configuration can be passed on the command line, and it makes these things more explicit.

I'm envisioning something like:

atlas-densities cell-densities fit-average-densities                                            \
    --hierarchy-path=data/1.json                                                                \
    --annotation-path=data/ccfv2/annotation_25.nrrd                                             \
    --neuron-density-path=data/ccfv2/density_volumes/neuron_density.nrrd                        \
    --average-densities-path=data/ccfv2/measurements/lit_densities.csv                          \
    --homogenous-regions-path=data/ccfv2/measurements/homogeneous_regions.csv                   \
    --marker=pv:868:PATH/TO/pvalb.nrrd \
    --marker=sst:1001:PATH/TO/SST.nrrd \
    --marker=vip:77371835:PATH/TO/VIP.nrrd \
    --marker=gad67:479:PATH/TO/gad1.nrrd \
    --realigned-slices=atlas_densities/app/data/markers/realigned_slices_ccfv2.json \
    --cell-density-standard-deviations=atlas_densities/app/data/measurements/std_cells.json \
    --fitted-densities-output-path=data/ccfv2/first_estimates/first_estimates.csv               \
    --fitting-maps-output-path=data/ccfv2/first_estimates/fitting.json
Sebastien-PILUSO commented 5 months ago

Can you point me to such a file?

Here is an example of the attached gene marker JSON file produced by the deep-atlas pipeline for the coronal dataset id 479 corresponding to gad67: /gpfs/bbp.cscs.ch/data/project/proj84/piluso/deep-atlas2/processes_large/gene-to-nissl/ccfv2/479-metadata.json

I think it would be good to move away from needing the fit_average_densities_ccfv2_config.json file, as all the same configuration can be passed on the command line, and it makes these things more explicit.

Absolutely.

I'm envisioning something like:

Great this would be valuable.

mgeplf commented 5 months ago

I made the change here: https://github.com/BlueBrain/atlas-densities/pull/67 Can you try it @Sebastien-PILUSO / @lecriste to see if it's what you expected?

Sebastien-PILUSO commented 5 months ago

The new command looks good. I think @drodarie and @lecriste should also have a look at it. I will soon re-run the atlas densities staff, so we would see if it works well.

drodarie commented 4 months ago

This does not change the behavior of the code, just how you pass parameters right? So you should not need my help / aproval on that one.

mgeplf commented 4 months ago

This does not change the behavior of the code, just how you pass parameters right?

That is correct, unless I have made a mistake.