theislab / moscot

Multi-omic single-cell optimal transport tools
https://moscot-tools.org
BSD 3-Clause "New" or "Revised" License
101 stars 9 forks source link

Add Sparse Geodesic Cost Support #677

Closed selmanozleyen closed 1 month ago

selmanozleyen commented 3 months ago

Here are the steps I planned. I first want to ensure the tests work with this version and I am currently on that step.

In order to comply to new version:

codecov[bot] commented 3 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 78.19%. Comparing base (3151da7) to head (f3c54be). Report is 25 commits behind head on main.

:exclamation: Current head f3c54be differs from pull request most recent head b054584

Please upload reports for the commit b054584 to get more accurate results.

Additional details and impacted files [![Impacted file tree graph](https://app.codecov.io/gh/theislab/moscot/pull/677/graphs/tree.svg?width=650&height=150&src=pr&token=Rgtm5Tsblo&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=theislab)](https://app.codecov.io/gh/theislab/moscot/pull/677?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=theislab) ```diff @@ Coverage Diff @@ ## main #677 +/- ## ======================================= Coverage 78.19% 78.19% ======================================= Files 36 36 Lines 3806 3806 Branches 706 706 ======================================= Hits 2976 2976 Misses 524 524 Partials 306 306 ``` [see 2 files with indirect coverage changes](https://app.codecov.io/gh/theislab/moscot/pull/677/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=theislab)
MUCDK commented 2 months ago

in the failing tests, set tau_a, tau_b > 0.5

giovp commented 2 months ago
selmanozleyen commented 2 months ago

I tried all of your suggestions and it only worked when I set the tau's to be 1.0 which is balanced. I checked again and it fails after this https://github.com/ott-jax/ott/commit/41906a2a1ade19aa154189fabd7c159a160c9bf3 commit specifically. I created a PR separately for the setting the defaults to None https://github.com/theislab/moscot/pull/686

giovp commented 2 months ago

ok then i think we need to raise this to @michalk8 because it is not expected

MUCDK commented 2 months ago

agree, can you maybe try to run the example without moscot, but directly with ott-jax @selmanozleyen ? This might help @michalk8

selmanozleyen commented 2 months ago

agree, can you maybe try to run the example without moscot, but directly with ott-jax @selmanozleyen ? This might help @michalk8

@MUCDK ok I created it here https://github.com/ott-jax/ott/issues/519