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

Nsetm 2196 synapse class #43

Merged
merged 66 commits into from
Aug 23, 2023
Merged

Nsetm 2196 synapse class #43

merged 66 commits into from
Aug 23, 2023

Conversation

edasubert
Copy link
Contributor

No description provided.

edasubert and others added 30 commits June 20, 2023 16:41
…TM-2196-update-create-from-probability-map-probability-map
…TM-2196-update-create-from-probability-map-probability-map
new command line:
    OUTPUT=output/create-from-probability-map
    DATA=../data
    atlas-densities  \
     mtype-densities -vv create-from-probability-map                              \
        --hierarchy-path=$DATA/1.json                                             \
        --annotation-path=$DATA/ccfv2/annotation_25.nrrd                          \
        --probability-map probability_map.csv                                     \
        --marker gad67 $DATA/create-from-probability-map/gad67.nrrd               \
        --marker sst $DATA/create-from-probability-map/sst.nrrd                   \
        --marker vip $DATA/create-from-probability-map/vip.nrrd                   \
        --marker approx_lamp5 $DATA/create-from-probability-map/approx_lamp5.nrrd \
        --marker pv $DATA/create-from-probability-map/pv.nrrd                     \
        --output-dir=$OUTPUT

Thus `--metadata-path` and `--mtypes-config-path` aren't used any more
Likely can be tuned better.
* speed up density creation

* :-|

---------

Co-authored-by: Mike Gevaert <michael.gevaert@epfl.ch>
* speed up density creation

* Stop using as many config files

new command line:
    OUTPUT=output/create-from-probability-map
    DATA=../data
    atlas-densities  \
     mtype-densities -vv create-from-probability-map                              \
        --hierarchy-path=$DATA/1.json                                             \
        --annotation-path=$DATA/ccfv2/annotation_25.nrrd                          \
        --probability-map probability_map.csv                                     \
        --marker gad67 $DATA/create-from-probability-map/gad67.nrrd               \
        --marker sst $DATA/create-from-probability-map/sst.nrrd                   \
        --marker vip $DATA/create-from-probability-map/vip.nrrd                   \
        --marker approx_lamp5 $DATA/create-from-probability-map/approx_lamp5.nrrd \
        --marker pv $DATA/create-from-probability-map/pv.nrrd                     \
        --output-dir=$OUTPUT

Thus `--metadata-path` and `--mtypes-config-path` aren't used any more

* :-|

* tests

* values should not change

* unique region acronyms

* :-|

---------

Co-authored-by: Mike Gevaert <michael.gevaert@epfl.ch>
…TM-2196-update-create-from-probability-map-probability-map
@codecov-commenter
Copy link

codecov-commenter commented Jul 27, 2023

Codecov Report

❗ No coverage uploaded for pull request base (main@9d0f9ce). Click here to learn what that means.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main      #43   +/-   ##
=======================================
  Coverage        ?   97.76%           
=======================================
  Files           ?       22           
  Lines           ?     1385           
  Branches        ?        0           
=======================================
  Hits            ?     1354           
  Misses          ?       31           
  Partials        ?        0           
Flag Coverage Δ
pytest 97.76% <0.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

@edasubert edasubert requested a review from mgeplf July 31, 2023 14:58
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think the existing synapses were inhibitory, no?

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 switched it around several times, you are more likely to be correct than me 😊

@@ -49,7 +59,7 @@ def _check_probability_map_consistency(
"""
check_probability_map_sanity(probability_map)

_, df_molecular_types = zip(*probability_map.index)
_, df_molecular_types, _ = zip(*probability_map.index)
Copy link
Collaborator

Choose a reason for hiding this comment

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

similar here: probability_map.index.get_level_values('molecular_types')?

@edasubert edasubert requested a review from mgeplf August 7, 2023 11:41
@edasubert
Copy link
Contributor Author

@mgeplf I think we should merge this one to make the densities usable

@edasubert edasubert merged commit 388d766 into main Aug 23, 2023
5 checks passed
@edasubert edasubert deleted the NSETM-2196-synapse-class branch August 23, 2023 14:12
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

Successfully merging this pull request may close these issues.

3 participants