DensMAP support - #2946
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2946 +/- ##
==========================================
+ Coverage 75.40% 75.44% +0.04%
==========================================
Files 113 114 +1
Lines 13259 13273 +14
==========================================
+ Hits 9998 10014 +16
+ Misses 3261 3259 -2
|
… into keller-mark/densmap-2
|
Hi, it seems the |
flying-sheep
left a comment
There was a problem hiding this comment.
Hi, thanks for you contribution! Please add a plot test that shows sufficiently different results from umap.
|
@keller-mark I'm curious if you're seeing locally what I'm seeing #2946 (comment) in the actual images produced. |
|
I do see the same actual/expected locally. I still cannot seem to find how to download the generated actual from Azure CI #2946 (comment) |
|
I think something along the lines of worked for me. Have you tried this? Maybe it's a permissions thing? |
|
Thanks @ilan-gold for getting the image artifacts working on Azure CI. I now see that the actual/expected for both umap and densmap differ there despite passing, even in the Python 3.12 environment. I believe having easy access to DensMAP in Scanpy will be valuable to the community and would love to see this feature merged. |
I would be happy with a numerical test |
|
I have added a numerical test to check that the mean of ellipse areas that result from fitting a Gaussian mixture model with N components on the 2D coordinates from DensMAP and UMAP differ, with the DensMAP ellipses being larger on average than the UMAP ones. Where N = number of Louvain clusters. Let me know if this makes sense or if you have a better idea for a numerical test that will check that the UMAP and DensMAP results differ. |
|
@keller-mark So the idea here is that a densemap always has slightly larger ellipses than UMAP, but is otherwise the same? This is the claim of the method? If so, the test looks alright to me |
Co-authored-by: Ilan Gold <ilanbassgold@gmail.com>
for more information, see https://pre-commit.ci
… into keller-mark/densmap-2
ilan-gold
left a comment
There was a problem hiding this comment.
I like the test - it says "this should look on average a certain way across different machines" which is about as good as we can hope for IMO. Unless there's something else, we can merge
|
Recent preprint discussing the benefits of DensMAP: https://doi.org/10.48550/arXiv.2506.08725 |
|
I have gotten this PR passing CI a few times but the underlying repo keeps changing out from under me, which is quite frustrating. I know it is a difficult problem and maintainers are spread thin but I truly believe having DensMAP functionality in Scanpy would benefit many users. |
I tried to follow the feedback described in a previous PR that contributed DensMAP #2684 (comment) but re-implemented on top of the state of the current scanpy main branch.
I did not add release notes because the contributor guide says to wait for PR feedback https://scanpy.readthedocs.io/en/latest/dev/documentation.html#adding-to-the-docs