Pbinder/task feat changes #404

polinabinder1 · 2025-09-16T19:07:49Z

Changes in the dataset and the task features.

mlgill · 2025-09-18T16:07:42Z

examples/test_equivalency_perturbation_dataset.py

@@ -0,0 +1,406 @@
+import argparse


Reviewer note: this will be removed before PR is merged.

mlgill · 2025-09-18T16:07:50Z

examples/test_equivalency_perturbation_task.py

@@ -0,0 +1,484 @@
+# This tests that the perturbation prediection task with the new data formats matches


Reviewer note: this will be removed before PR is merged.

mlgill

Some initial comments. Will test your branch later today and add additional feedback.

src/czbenchmarks/tasks/single_cell/perturbation_expression_prediction.py

mlgill · 2025-09-18T16:14:44Z

src/czbenchmarks/tasks/single_cell/perturbation_expression_prediction.py

+        self.adata.var.index = model_adata.var.index
+
+        # Apply cell barcode ordering
+        self.adata.uns["cell_barcode_index"] = model_adata.obs.index.astype(str).values


Should check for the existence of this key when the file is opened. Please add the expectations for data to the planned documentation updates.

added and will add

Unresolving as a reminder for the documentation addition

src/czbenchmarks/tasks/single_cell/perturbation_expression_prediction.py

mlgill · 2025-09-18T16:21:25Z

src/czbenchmarks/tasks/single_cell/perturbation_expression_prediction.py

+            pred_lfc = cell_representation[np.ix_(condition_idx, gene_indices)].mean(
+                axis=0
+            ) - cell_representation[np.ix_(control_idx, gene_indices)].mean(axis=0)
+


Can you add a comment here to me to ensure we check that the input data do not look like counts (i.e. no fractional components)?

Actually, this may be something for you to pick up if you have time. Let's discuss on Friday after I speak with Laksshman today.

Left a comment on this here: https://github.com/chanzuckerberg/cz-benchmarks/pull/404/files#r2372452575

changed it!

tests/datasets/test_single_cell_perturbation_dataset.py

tests/test_integration_end_to_end.py

mlgill · 2025-09-19T14:09:59Z

examples/example_perturbation_expression_prediction.py

        default=0.55,
        help="Minimum standardized mean difference for DE filtering (used when --metric=t-test)",
    )
+    parser.add_argument(


For the help under "metric", could we list the two possibilities?

Also, all default values should match what the default is set to in the respective method.

I'm pretty sure the values match the defaults

Percent genes to mask does not mask (that's the one I was looking at when I wrote this). The rest are indeed idential. In the dataset class:
percent_genes_to_mask: float = 0.5

Also, metric should be removed as an arg from the script -- it's been deleted from the dataset/task since there is only one possibility right now.

examples/example_perturbation_expression_prediction.py

mlgill · 2025-09-19T14:34:36Z

examples/example_perturbation_expression_prediction.py

+        "The file should have: cell representations in .X, gene names in .var.index, "
+        "and cell identifiers in .obs.index. "
+        "The gene names and cell identifiers should match the task input, although the ordering does not need to be the same.",
+    )


Line 113 notes

TODO: Once PR 381 is merged, use the new load_local_dataset function

PR 381 has been merged. Can this be done or should this comment be removed?

removed, thanks for catching it.

What was the resolution here? Is it not possible to use the new function?

examples/example_perturbation_expression_prediction.py

src/czbenchmarks/tasks/single_cell/perturbation_expression_prediction.py

mlgill · 2025-09-19T14:59:12Z

src/czbenchmarks/tasks/single_cell/perturbation_expression_prediction.py

-    This creates a PerturbationExpressionPredictionTaskInput from stored files,
-    allowing the task to be instantiated without going through the full dataset
-    loading process.
+    Load perturbation task inputs from saved separate files.


Do we still need this function now that the output artifacts have been simplified?

It could be useful to fully process the dataset, then to run the tasks. (This saves time, and ensures consistency if there's random sampling)

Cool. If we're going to keep it, let's do a quick check for "cell_barcode_condition_index" in adata.uns since it's a direct input to the task class. (I realize it's also done in validate, but that won't catch this one.)

src/czbenchmarks/tasks/single_cell/perturbation_expression_prediction.py

mlgill · 2025-09-19T15:02:24Z

src/czbenchmarks/tasks/single_cell/perturbation_expression_prediction.py

    def __init__(
        self,
        metric: str = "wilcoxon",
        control_prefix: str = "non-targeting",


Please use nomenclature and default value from the dataset class:

control_name: str = "ctrl"

https://github.com/chanzuckerberg/cz-benchmarks/blob/main/src/czbenchmarks/datasets/single_cell_perturbation.py#L92

In the example script, we can use the hydra config values from the dataset yaml config to set control_prefix and condition.

Look like it's non-targeting everywhere including in the dataset.yaml

src/czbenchmarks/tasks/single_cell/perturbation_expression_prediction.py

mlgill · 2025-09-19T17:02:32Z

src/czbenchmarks/datasets/single_cell_perturbation.py

        Validates the following:
        - Condition format must be one of:
-          - ``{control_name}`` or ``{control_name}_{perturb}`` for matched control samples.
+          - ``{control_name}_{perturb}`` for matched control samples.


IIRC, this should run on the data before it's control matched too, so I think we need to leave the ability to match against just the control_name and update the end of this to say "unmatched or matched control samples". Does that sound right?

src/czbenchmarks/datasets/single_cell_perturbation.py

mlgill · 2025-09-23T14:02:51Z

src/czbenchmarks/tasks/utils.py

    return adata.obsm[obsm_key]
+
+
+def guess_is_lognorm(


Just thought of this -- it looks very similar to the other repo, including the title. If so we will need to do SWIPAT checks before release, which would impact our release. I think there are other ways to do this check that might not require that (and we should change the function name).

changed it up!

mlgill · 2025-09-23T14:16:38Z

.github/workflows/publish-pypi.yml

        run: |
          echo "VERSION=$(uv version --short)" >> $GITHUB_OUTPUT

      - name: Display version being published


Also looks like the merge didn't quite work -- I had this issue with my PR too. I'd merged, but for some reason github didn't detect it. I had to do the merge within the PR on GitHub even though it was trivial.

polinabinder1 requested a review from mlgill September 16, 2025 19:07

polinabinder1 changed the base branch from main to michelle/de_checks September 16, 2025 19:08

polinabinder1 force-pushed the pbinder/task_feat_changes branch from 1f50514 to a75a668 Compare September 17, 2025 19:33

polinabinder1 added 8 commits September 17, 2025 21:18

adding target conditions

d4a8804

fixing test cases

a06f892

more test fixes

1a28368

all tests pass

3391383

outputting a single h5ad file

e6bb224

running tests

e7b2a0f

different data format

eb7f135

error logging and better format of the example

cb77ff1

polinabinder1 force-pushed the pbinder/task_feat_changes branch from 09ca021 to cb77ff1 Compare September 18, 2025 04:18

end to end test

e39b748

mlgill reviewed Sep 18, 2025

View reviewed changes

some simple test fixes

19de04c

mlgill reviewed Sep 19, 2025

View reviewed changes

examples/example_perturbation_expression_prediction.py Outdated Show resolved Hide resolved

mlgill reviewed Sep 19, 2025

View reviewed changes

examples/example_perturbation_expression_prediction.py Outdated Show resolved Hide resolved

mlgill reviewed Sep 19, 2025

View reviewed changes

src/czbenchmarks/tasks/single_cell/perturbation_expression_prediction.py Outdated Show resolved Hide resolved

mlgill reviewed Sep 19, 2025

View reviewed changes

src/czbenchmarks/tasks/single_cell/perturbation_expression_prediction.py Show resolved Hide resolved

mlgill reviewed Sep 19, 2025

View reviewed changes

src/czbenchmarks/tasks/single_cell/perturbation_expression_prediction.py Outdated Show resolved Hide resolved

mlgill reviewed Sep 19, 2025

View reviewed changes

src/czbenchmarks/datasets/single_cell_perturbation.py Outdated Show resolved Hide resolved

mlgill reviewed Sep 19, 2025

View reviewed changes

src/czbenchmarks/datasets/single_cell_perturbation.py Outdated Show resolved Hide resolved

addressing PR comments

c54d8dd

polinabinder1 added 5 commits September 19, 2025 16:20

adding util file

b20b4b5

fully adressing comments

654a0f1

Merge remote-tracking branch 'origin' into pbinder/task_feat_changes

b02ec56

branch merge + test update

71d1bbc

adding some documentation

1be30aa

mlgill reviewed Sep 23, 2025

View reviewed changes

polinabinder1 added 5 commits September 23, 2025 13:40

addressing the PR

4dcf186

merging changes

9f40130

reverting a change

3c539e1

doing the merge

72c0816

fixing removed

93de3e3

		@@ -0,0 +1,484 @@
		# This tests that the perturbation prediection task with the new data formats matches

Pbinder/task feat changes #404

Are you sure you want to change the base?

Pbinder/task feat changes #404

Conversation

polinabinder1 commented Sep 16, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mlgill left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mlgill Sep 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mlgill Sep 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mlgill Sep 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mlgill Sep 23, 2025 •

edited

Loading

mlgill Sep 19, 2025 •

edited

Loading

mlgill Sep 19, 2025 •

edited

Loading