-
Notifications
You must be signed in to change notification settings - Fork 4
Improve metrics documentation #204
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
base: main
Are you sure you want to change the base?
Conversation
for more information, see https://pre-commit.ci
src/cellflow/metrics/_metrics.py
Outdated
@@ -54,15 +94,46 @@ def pairwise_squeuclidean(x: ArrayLike, y: ArrayLike) -> ArrayLike: | |||
|
|||
@jax.jit | |||
def compute_e_distance_fast(x: ArrayLike, y: ArrayLike) -> float: | |||
"""Compute the energy distance as in Peidli et al.""" | |||
"""Compute the energy distance between x and y as in Peidli et al. |
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.
make proper reference with :cite:
, you might have to add the reference first.
src/cellflow/metrics/_metrics.py
Outdated
) | ||
|
||
|
||
def compute_e_distance(x: ArrayLike, y: ArrayLike) -> float: | ||
"""Compute the energy distance as in Peidli et al.""" | ||
"""Compute the energy distance between x and y as in Peidli et al. |
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.
Add proper reference
src/cellflow/metrics/_metrics.py
Outdated
return r2_score(np.mean(x, axis=0), np.mean(y, axis=0)) | ||
|
||
|
||
def compute_sinkhorn_div(x: ArrayLike, y: ArrayLike, epsilon: float = 1e-2) -> float: | ||
"""Compute the Sinkhorn divergence between x and y.""" | ||
"""Compute the Sinkhorn divergence between x and y. |
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.
cite feydy 2019 (https://proceedings.mlr.press/v89/feydy19a.html) as a ref
src/cellflow/metrics/_metrics.py
Outdated
@@ -21,12 +23,38 @@ | |||
|
|||
|
|||
def compute_r_squared(x: ArrayLike, y: ArrayLike) -> float: | |||
"""Compute the R squared between true (x) and predicted (y)""" | |||
"""Compute the R squared score between the true (x) and predicted (y) distributions. |
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.
It's actually R squared between the means of x
and y
, please say so
Returns | ||
------- | ||
A scalar denoting the average MMD over all gammas. | ||
""" | ||
if gammas is None: | ||
gammas = [2, 1, 0.5, 0.1, 0.01, 0.005] | ||
mmds = [maximum_mean_discrepancy(x, y, gamma=gamma) for gamma in gammas] # type: ignore[union-attr] | ||
return np.nanmean(np.array(mmds)) | ||
|
||
|
||
def compute_metrics_fast(x: ArrayLike, y: ArrayLike) -> dict[str, float]: |
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 we expose it, but not add to the docs? to keep it for backward compatibility ,but it's a bit redundant.
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.
see comments
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #204 +/- ##
==========================================
- Coverage 79.88% 77.13% -2.76%
==========================================
Files 38 38
Lines 2491 2497 +6
Branches 313 313
==========================================
- Hits 1990 1926 -64
- Misses 362 444 +82
+ Partials 139 127 -12
🚀 New features to boost your workflow:
|
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 changes pls, other than that looks good
return r2_score(np.mean(x, axis=0), np.mean(y, axis=0)) | ||
|
||
|
||
def compute_sinkhorn_div(x: ArrayLike, y: ArrayLike, epsilon: float = 1e-2) -> float: | ||
"""Compute the Sinkhorn divergence between x and y.""" | ||
"""Compute the Sinkhorn divergence between x and y as in Feydy et al. :cite:`feydy:19`. |
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.
Just cite once, i.e. Compute the Sinkhorn divergence between x and y as in :cite:
feydy:19`.
Similarly in other places, please.
No description provided.