Debug h5ad saving #580

Sichao25 · 2023-09-29T17:20:36Z

Some parts of adata still have incompatible data types. Including:

fate output -> convert to a different format when saving, convert back when loading
kmc object -> convert to a different format when saving, convert back when loading
umap object -> now umap object will not be saved, it will be constructed with parameters when needed.
velocity parameters -> convert to compatible data type
cell phase genes -> convert to compatible data type
init cells -> convert to compatible data type
fixed points and nullclines -> convert to compatible data type, nullclines are replaced by NCx and NCy

In the future, we will use import_h5ad and export_h5ad for saving and loading processed data.

codecov-commenter · 2023-09-29T17:27:57Z

Codecov Report

Attention: 79 lines in your changes are missing coverage. Please review.

Comparison is base (e5e9017) 22.51% compared to head (8f3cc0f) 22.13%.
Report is 139 commits behind head on master.

Files	Patch %	Lines
dynamo/data_io.py	15.38%	33 Missing ⚠️
dynamo/utils.py	17.64%	14 Missing ⚠️
dynamo/tools/connectivity.py	13.33%	13 Missing ⚠️
dynamo/tools/cell_velocities.py	22.22%	7 Missing ⚠️
dynamo/tools/markers.py	0.00%	4 Missing ⚠️
dynamo/plot/topography.py	0.00%	3 Missing ⚠️
dynamo/prediction/fate.py	40.00%	3 Missing ⚠️
dynamo/tools/utils.py	50.00%	1 Missing ⚠️
dynamo/vectorfield/topography.py	0.00%	1 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #580      +/-   ##
==========================================
- Coverage   22.51%   22.13%   -0.38%     
==========================================
  Files         165      166       +1     
  Lines       26991    28043    +1052     
==========================================
+ Hits         6077     6208     +131     
- Misses      20914    21835     +921

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Xiaojieqiu

Thanks Sichao, I made some comments. we can briefly discuss about it

Xiaojieqiu · 2023-10-06T21:14:33Z

dynamo/data_io.py

+def export_kmc(adata: AnnData) -> None:
+    """Save the parameters of kmc and delete the kmc object from anndata."""
+    kmc = adata.uns["kmc"]
+    adata.uns["kmc_params"] = {
+        "P": kmc.P,
+        "Idx": kmc.Idx,
+        "eignum": kmc.eignum,
+        "D": kmc.D,
+        "U": kmc.U,
+        "W": kmc.W,
+        "W_inv": kmc.W_inv,
+        "Kd": kmc.Kd,
+    }
+    adata.uns.pop("kmc")


why not add this to the place where kmc is saved to adata.uns?

In this way, we can keep a kmc object when we run the Dynamo analysis. If we need it, we just read it from the adata.uns.

Umap object uses a different saving strategy, which means umap parameters will be saved instead of the object itself. My idea is that it is possible to avoid creating a umap in the pipeline even if we run the umap dimension reduction. Unless the user wants to perform inverse transform in a specific analysis like fate, they don't need this Umap instance. While the creation of KMC is inevitable if the user enables the kmc method in tl.cell_velocities. Since we have already created KMC, we can keep it for future usage until saving to h5ad.

dynamo/data_io.py

Xiaojieqiu · 2023-10-06T21:24:07Z

dynamo/plot/topography.py

+        NCx, NCy = (
+            [vecfld_dict["NCx"][index] for index in vecfld_dict["NCx"]],
+            [vecfld_dict["NCy"][index] for index in vecfld_dict["NCy"]],


what will be the behaviors for this? will this reorder the x/y coordinates of the nullclines?
[vecfld_dict["NCx"][index] for index in vecfld_dict["NCx"]],
[vecfld_dict["NCy"][index] for index in vecfld_dict["NCy"]],

This will read the nullclines from the dictionary to form a list of x and y coordinates. The order should be the same. Here is a reference indicating regular dictionaries have kept their items in the same order that they were inserted into the underlying dictionary since python 3.6.

Xiaojieqiu · 2023-10-06T21:25:58Z

dynamo/prediction/fate.py

+            negative_sample_rate=params["umap_kwargs"]["negative_sample_rate"],
+            init_pos=params["umap_kwargs"]["init_pos"],
+            random_state=params["umap_kwargs"]["random_state"],
+            umap_kwargs=params["umap_kwargs"],


umap_kwargs=params["umap_kwargs"]
will this lead to pass duplicated arguments to construct_mapper_umap?

params["umap_kwargs"] may pass duplicate arguments. But this should not raise an error because in construct_mapper_umap we use update_dict.

Xiaojieqiu · 2023-10-06T21:26:11Z

dynamo/prediction/fate.py

+            negative_sample_rate=params["umap_kwargs"]["negative_sample_rate"],
+            init_pos=params["umap_kwargs"]["init_pos"],
+            random_state=params["umap_kwargs"]["random_state"],
+            umap_kwargs=params["umap_kwargs"],


umap_kwargs=params["umap_kwargs"]
will this lead to pass duplicated arguments to construct_mapper_umap?

params["umap_kwargs"] may pass duplicate arguments. But this should not raise an error because in construct_mapper_umap we use update_dict.

Xiaojieqiu · 2023-10-06T21:28:09Z

dynamo/prediction/fate.py

        "average": average,
        "t": t,
        "prediction": prediction,
        # "VecFld": VecFld,
-        "VecFld_true": VecFld_true,
+        # "VecFld_true": VecFld_true,


VecFld_true means the groundtruth vector field.

Will we need this in the pipeline?

Xiaojieqiu · 2023-10-06T21:28:32Z

dynamo/prediction/least_action_path.py

        "init_states": init_states,
-        "init_cells": init_cells,
+        "init_cells": list(init_cells),


why list only applies to init_cells instead of init_states?

init_cells can be an Index (so we convert it to a list) while init_states is an array (a more compatible data type).

dynamo/preprocessing/cell_cycle.py

dynamo/preprocessing/pca.py

Sichao25 added 17 commits July 7, 2023 17:47

create conversion func

e6915f6

add docstr

5cd67ec

debug saving cell_phase_genes

05ec638

debug saving markers

52b7128

debug velocity params saving

7018bc1

create umap object construction func

902ac4b

call umap construct

e092c80

update fate umap transform

29d0d66

remove umap saving

b681814

debug nullcline saving

ffa41cd

refactor pca_fit transform

4ff1eec

add docstr for helper func

e70f895

debug fate saving

321cc8c

create import func and debug prediction saving

36a8591

create kmc import export

2ef5e0c

debug h5ad main saving function

02af0f2

add docstr for h5ad saving

8d1cc70

Sichao25 marked this pull request as ready for review September 29, 2023 18:02

Xiaojieqiu reviewed Oct 6, 2023

View reviewed changes

Sichao25 and others added 6 commits October 10, 2023 11:29

merge duplicate functions

97b279b

delete duplicate funcs

6ec2fd5

Merge branch 'master' into debug_l

514a2a6

update data saving after merging master

0a5e5d9

Merge branch 'master' into debug_l

12cdc52

revert the deletion of fate results save

8f3cc0f

Xiaojieqiu merged commit 8456e3b into aristoteleo:master Nov 16, 2023
5 checks passed

Sichao25 mentioned this pull request Nov 29, 2023

save h5ad error [TypeError: Can't implicitly convert non-string objects to strings] #622

Closed

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Debug h5ad saving #580

Debug h5ad saving #580

Sichao25 commented Sep 29, 2023 •

edited

Loading

codecov-commenter commented Sep 29, 2023 •

edited

Loading

Xiaojieqiu left a comment

Xiaojieqiu Oct 6, 2023

Sichao25 Oct 9, 2023

Xiaojieqiu Oct 6, 2023

Sichao25 Oct 9, 2023

Xiaojieqiu Oct 6, 2023

Sichao25 Oct 9, 2023

Xiaojieqiu Oct 6, 2023

Sichao25 Oct 9, 2023

Xiaojieqiu Oct 6, 2023

Sichao25 Oct 10, 2023

Xiaojieqiu Oct 6, 2023

Sichao25 Oct 9, 2023

Debug h5ad saving #580

Debug h5ad saving #580

Conversation

Sichao25 commented Sep 29, 2023 • edited Loading

codecov-commenter commented Sep 29, 2023 • edited Loading

Codecov Report

Xiaojieqiu left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Sichao25 commented Sep 29, 2023 •

edited

Loading

codecov-commenter commented Sep 29, 2023 •

edited

Loading