Skip to content
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.

Sign up for GitHub

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

Jump to bottom

elem_mul in variance calculation should use float64 casting #3127

Open
3 tasks done
ilan-gold opened this issue Jun 26, 2024 · 0 comments · May be fixed by #3392
Open
3 tasks done

elem_mul in variance calculation should use float64 casting #3127

ilan-gold opened this issue Jun 26, 2024 · 0 comments · May be fixed by #3392
Assignees
@ilan-gold
Milestone
1.12.0

Comments

@ilan-gold
Copy link
Contributor

ilan-gold commented Jun 26, 2024 •
edited
Loading

Please make sure these conditions are met

  • I have checked that this issue has not already been reported.
  • I have confirmed this bug exists on the latest version of scanpy.
  • (optional) I have confirmed this bug exists on the main branch of scanpy.

What happened?

When we calculate X*X for variance, we preserve the data type of the incoming X, but this actually can cause downstream inaccuracies from overflow differences. This has been the case for many years

Really we should do something like np.multiply(X, X, dtype="float64). This would be more accurate/sensible. This came up in the context of https://github.com/scverse/scanpy/pull/3099/files#diff-afb2fb35cbde7ff5e7d9b79874ede22605918cdba923250dd554f23353702e45R65-R67 where @Intron7 was casting first, and then multiplying (because it should be more accurate), but this revealed that we are not doing this at the moment, despite the fact that it is more accurate. And then downstream analyses can change quite a bit.

Thus we should remedy this for the next minor release as it is a breaking change to e.g., https://dev.azure.com/scverse/scanpy/_build/results?buildId=7094&view=logs&j=5ea502cf-d418-510c-3b5f-c4ba606ae534&t=534778bb-2f86-5739-7d3c-59518f7b5a2b&l=2171
Screenshot 2024-06-26 at 16 21 27

Minimal code sample

import numpy as np
arr = np.random.random((10, 10_000)).astype("float32")

print(np.multiply(arr, arr, dtype="float64"))
print(np.multiply(arr, arr))

Error output

N/A

Versions

-----
anndata     0.10.7
scanpy      1.10.0rc2.dev74+g1c98fd19
-----
IPython             8.24.0
PIL                 10.3.0
asciitree           NA
asttokens           NA
cloudpickle         3.0.0
cycler              0.12.1
cython_runtime      NA
dask                2024.5.1
dateutil            2.9.0.post0
decorator           5.1.1
defusedxml          0.7.1
distutils           3.12.3
executing           2.0.1
h5py                3.11.0
igraph              0.11.5
jedi                0.19.1
jinja2              3.1.4
joblib              1.4.2
kiwisolver          1.4.5
legacy_api_wrap     NA
leidenalg           0.10.2
llvmlite            0.42.0
louvain             0.8.2
markupsafe          2.1.5
matplotlib          3.9.0
mpl_toolkits        NA
msgpack             1.0.8
natsort             8.4.0
numba               0.59.1
numcodecs           0.12.1
numpy               1.26.4
packaging           24.0
pandas              2.2.2
parso               0.8.4
pkg_resources       NA
prompt_toolkit      3.0.45
psutil              5.9.8
pure_eval           0.2.2
pyarrow             16.1.0
pygments            2.18.0
pyparsing           3.1.2
pytz                2024.1
scipy               1.13.1
session_info        1.0.0
setuptools          70.0.0
setuptools_scm      NA
sitecustomize       NA
six                 1.16.0
sklearn             1.5.0
sparse              0.15.4
sphinxcontrib       NA
stack_data          0.6.3
tblib               3.0.0
texttable           1.7.0
threadpoolctl       3.5.0
tlz                 0.12.1
toolz               0.12.1
traitlets           5.14.3
wcwidth             0.2.13
yaml                6.0.1
zarr                2.18.2
-----
Python 3.12.3 (main, Apr  9 2024, 08:09:14) [Clang 15.0.0 (clang-1500.1.0.2.5)]
macOS-13.6.1-arm64-arm-64bit
-----
Session information updated at 2024-06-26 16:19
@ilan-gold ilan-gold added Bug 🐛 Triage 🩺 This issue needs to be triaged by a maintainer labels Jun 26, 2024
@ilan-gold ilan-gold added this to the 1.11.0 milestone Jun 26, 2024
@flying-sheep flying-sheep removed the Triage 🩺 This issue needs to be triaged by a maintainer label Oct 21, 2024
@ilan-gold ilan-gold self-assigned this Nov 21, 2024
@ilan-gold ilan-gold linked a pull request Dec 5, 2024 that will close this issue
Open
3 tasks
@flying-sheep flying-sheep modified the milestones: 1.11.0, 1.12.0 Dec 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@ilan-gold ilan-gold

Labels
None yet
Projects
None yet
Milestone
1.12.0
Development

Successfully merging a pull request may close this issue.

(feat): elem_mul 64 bit upcast option scverse/scanpy
2 participants
@flying-sheep @ilan-gold