[WIP] Add functions for query-target distance calculations #17
Conversation
schlegelp
changed the title
schlegelp
changed the title
| gw_coupling_mat_csv: Optional[str] = None, | ||
| return_coupling_mats: bool = False, | ||
| ) -> tuple[ | ||
| DistanceMatrix, # GW distance matrix (Squareform) |
There was a problem hiding this comment.
should be
Matrix, # GW distance matrix (not necessarily a square)
|
|
||
| :return: If `return_coupling_mats` is True, | ||
| returns `( gw_dmat, couplings )`, | ||
| where gw_dmat is a square matrix whose (i,j) entry is the GW distance |
There was a problem hiding this comment.
gw_dmat is not necessarily square
| raise Exception( | ||
| "Must supply list of cell identifiers for writing to file." | ||
| ) | ||
| gw_data = csv_output_writer( |
There was a problem hiding this comment.
It's nice that you were able to make use of this existing function. When I wrote it I had square matrices in mind but I think this should be correct.
| return_coupling_mats: bool = False, | ||
| verbose: Optional[bool] = False, | ||
| ) -> tuple[ | ||
| DistanceMatrix, # Pairwise GW distance matrix (Squareform) |
There was a problem hiding this comment.
Matrix, # Pairwise GW distance matrix (need not be square)
|
Hi Philipp, I don't have any comments on the code but there are a few changes to the docstrings and type labels I suggested. The way the code currently stands, the type We can rename the type to something else to make it easier to distinguish between square symmetric distance matrices and general rectangular distance matrices, but on the other hand the distance matrices in the query/target code don't have any special properties other than being nonnegative, so perhaps they need a special name. For future work, I don't think it's necessary (for computational reasons, at least) to do something like this for the SLB distances, which I expect to be sufficiently fast to compute that one can just do all-by-all comparisons and then filter afterwards. Although the SLB is itself a poor estimator of GW, in its own right it seems to give reasonably good performance on our k-nn classifier tests, so it may be of direct use. On the other hand, I do think that implementing the query version of the QGW is likely to be useful, and the design can be basically identical to the design of this one. It would be nice to unify some of this redundant code but I am not sure that the benefits of a uniform interface would outweigh the increased complexity (in Python, at least). Again, thank you for this contribution! |
Hi! This PR would add functions to run query->target distance calculations, complementing the current all-by-all functionality.
The use-case for these (at least in my work) would be to find, for each query neuron, the best match(es) among a pool of target neurons.
TODOs:
Before I continue working on the TODOs above, I was wondering if you think these would be a good fit? And if you do, whether you agree with the way it is implemented in this first pass?