DensMAP support

[keller-mark]

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

• Closes densmap #1619
• Closes DensMAP support #2684
• Tests included or not required because:

• Release notes not necessary because:

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 75.44%. Comparing base (607bb9c) to head (019035d).

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     

Files with missing lines	Coverage Δ
src/scanpy/tools/__init__.py	91.30% <ø> (ø)
src/scanpy/tools/_types.py	100.00% <100.00%> (ø)
src/scanpy/tools/_umap.py	75.71% <100.00%> (+6.35%)	⬆️

[keller-mark]

Hi, it seems the Check if milestone or "no milestone" label is present GH Actions check fails but I am not able to add a PR label

[flying-sheep]

Hi, thanks for you contribution! Please add a plot test that shows sufficiently different results from umap.

[ilan-gold]

Specifically going from numba 0.60.0 to 0.61.0 does it

[keller-mark]

Thank you very much for debugging this @ilan-gold . Should I add a flag to skip this image comparison test with numba < 0.61.0?

[ilan-gold]

For now let's see if that makes things go green here and in the meantime I'll do some more digging.

[ilan-gold]

@keller-mark Something interesting is that numba 0.61.0 did not exist until a week ago, but your densemap issue predates that. So either that is not the issue, or I am misreading the history of this issue.

[ilan-gold]

@keller-mark I think that like most plotting tests in this repo (unfortunately), this test is actually erroneously passing with latest deps (including latest numba, for reasons I don't understand), at least for me locally. Here are actual/expected for me locally that "pass" as densemap:

[ilan-gold]

@keller-mark I'm curious if you're seeing locally what I'm seeing #2946 (comment) in the actual images produced.

[keller-mark]

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)

[ilan-gold]

I think something along the lines of

  - task: PublishBuildArtifacts@1
    inputs:
      pathToPublish: '$(Pipeline.Workspace)/s/scanpy/tests/_images'
      artifactName: drop

worked for me. Have you tried this? Maybe it's a permissions thing?

[keller-mark]

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.
Based on the discussion in #3422 (comment) I think it is unreasonable to expect random results, even with the same seed, to match between my local environment and the CI environment. Therefore I reiterate my earlier request to replace the visual test with a numerical test #2946 (comment) Alternatively, we could store a precomputed DensMAP result and plot that, but this would just test the sc.pl.embedding code path which is already tested and untouched in this PR.

[ilan-gold]

Since ultimately the plotting is just using the standard sc.pl.embedding, is a visual test really necessary here? That code path is already extensively tested. Instead I could add a numerical (as opposed to visual) test that checks that the method="densmap" produces sufficiently different results than method="umap"

I would be happy with a numerical test

[keller-mark]

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.

[ilan-gold]

@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

[ilan-gold]

Looking good

[ilan-gold]

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