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.

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

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

Open
Sebastien-PILUSO opened this issue Feb 16, 2024 · 6 comments

Comments

@Sebastien-PILUSO
Copy link

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

mgeplf commented Feb 19, 2024

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

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 added a commit that referenced this issue Feb 20, 2024
…verage-densities` command

* The updated command doesn't require a combine_markers_ccfv2_config.yaml
* Instead, the command line has:
    --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 \
* As discussed in:
    #66
@mgeplf
Copy link
Collaborator

mgeplf commented Feb 20, 2024

I made the change here:
#67
Can you try it @Sebastien-PILUSO / @lecriste to see if it's what you expected?

@Sebastien-PILUSO
Copy link
Author

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

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

mgeplf commented Feb 22, 2024

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

That is correct, unless I have made a mistake.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants