-
Notifications
You must be signed in to change notification settings - Fork 1
Initial mapping for IHCs and SGNs #42
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
Conversation
constantinpape
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I went through this and left a few minor comments. For now, two things are important:
-
Refactor the
tonotopic_mappingfunction so that path fitting and run-length estimation are separated from the tonotopic mapping (= applying the greenwood function to the run-length). In addition, we should also calculate the distance to the center of the path here, with distance=0 encoding "on path". -
Implement an updated version of the runlength mapping that respects the "centrality" of the path. This is especially important for SGNs. I asked ChatGPT for recommendations, and a distance weighted Steiner Tree sounds very feasible. Here is the detailed recommendations: https://chatgpt.com/share/6873d86c-cc90-8000-be74-db93bab8b968
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor comment, but we could create a new submodule measurement for this, and then put this into a file called tonotopic mapping. We can do this later, as I want to re-organize the code a bit anyways.
| max_dist = 0 | ||
| farthest_pair = (None, None) | ||
|
|
||
| for u, dist_dict in all_lengths.items(): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor comment: it should be possible to vectorize this by mappling the result from all_length to a numpy array and then using np.argmax. (This is likely not a bottleneck, so doesn't really matter that much.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The bottleneck of this function is nx.all_pairs_dijkstra_path_length(G, weight=weight), so I will try to find a faster alternative if I have time.
| unfiltered_graph = graph.copy() | ||
|
|
||
| if filter_factor is not None: | ||
| if 0 <= filter_factor < 1: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor comment: in general it would be safer to use a "spatially stratified" sample here. (For the current application in SGNs the random sample is probably fine though). Here are some suggestions from ChatGPT: https://chatgpt.com/share/6873d42d-d99c-8000-a0d3-b56efbd69ec8
|
|
||
| u, v = find_most_distant_nodes(graph) | ||
|
|
||
| if not nx.has_path(graph, source=u, target=v) or cell_type == "ihc": |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you explain the logic between the two branches here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The idea behind the two branches were the different use cases of IHC and SGN segmentation. While the Steiner Tree made sense for the IHC, where we expect a single line of cells, the application of the shortest path, which might skip intermediate cells, if they lie below the maximal edge distance of edges between nodes, seemed logical for SGNs. However, I can see that the distinction is probably not necessary, especially with the upcoming incorporation of the centrality for SGNs.
| for key in list(path_list.keys()): | ||
| tonotopic[int(path_list[key]["label_id"] - 1)] = path_list[key]["tonotopic"] * total_distance | ||
|
|
||
| table.loc[:, "tonotopic_label"] = tonotopic |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think everything until here should be refactored into the function that measures the run-length across the helix.
| for key in list(path_list.keys()): | ||
| tonotopic_map[int(path_list[key]["label_id"] - 1)] = var_A * (var_exp ** path_list[key]["tonotopic"] - var_k) | ||
|
|
||
| table.loc[:, "tonotopic_value[kHz]"] = tonotopic_map |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And this part then becomes the other function for tonotopic mapping.
|
The last commit addressed most of the comments by creating separate functions to measure the run length of the cochlea for SGNs and IHCs , by using "spatially stratified" subsampling with a voxel grid, and by creating a function to assign frequency ranges based on tonotopic mapping. |
|
This looks good now. We still need to refactor this at some point, but let's wait till all experiments are finalized with this. |
The goal of this branch is to introduce the tonotopic mapping of SGNs and IHCs. The cells are mapped onto the length of the cochlea's spiral structure. This is achieved by determining the shortest path between the two most distant nodes within the cochlear component of the segmentation.
More intricate mappings, such as mapping the centrality of SGNs, have yet to be implemented.
WIP