Skip to content

add destvi dataset - #526

Merged
scottgigante-immunai merged 30 commits into
openproblems-bio:mainfrom
giovp:dataset/spatial_decomposition/destvi
Oct 12, 2022
Merged

add destvi dataset#526
scottgigante-immunai merged 30 commits into
openproblems-bio:mainfrom
giovp:dataset/spatial_decomposition/destvi

Conversation

@giovp

@giovp giovp commented Jul 31, 2022

Copy link
Copy Markdown
Collaborator

Submission type

  • This submission adds a new dataset
  • This submission adds a new method
  • This submission adds a new metric
  • This submission adds a new task
  • This submission adds a new Docker image
  • This submission fixes a bug (link to related issue: )
  • This submission adds a new feature not listed above

Testing

  • This submission was written on a forked copy of SingleCellOpenProblems
  • GitHub Actions "Run Benchmark" tests are passing on this base branch of this pull
    request (include link to passed test: )
  • If this pull request is not ready for review (including passing the "Run
    Benchmark" tests), I will open this PR as a draft (click on the down arrow next to the
    "Create Pull Request" button)

Submission guidelines

  • This submission follows the guidelines in our
    Contributing document
  • I have checked to ensure there aren't other open Pull Requests for the
    same update/change

PR review checklist

This PR will be evaluated on the basis of the following checks:

  • The task addresses a valid open problem in single-cell analysis
  • The latest version of master is merged and tested
  • The methods/metrics are imported to __init__.py and were tested in the pipeline
  • Method and metric decorators are annotated with paper title, year, author, code
    version, and date
  • The README gives an outline of the methods, metrics and datasets in the folder
  • The README provides a satisfactory task explanation (for new tasks)
  • The sample test data is appropriate to test implementation of all methods and
    metrics (for new tasks)

@giovp

giovp commented Jul 31, 2022

Copy link
Copy Markdown
Collaborator Author

hi @scottgigante-immunai ,
I'd like to finish up the spatial decomposition task with an additional dataset from destvi paper. Two things to discuss:

  • the data generation requires 2 files (pickled). I understand from previous PR you asked to host them somewhere with a URL. However, I couyldn't come up with a good place to store them, furthermore sometime URL requests fails and break CI temporarily (recurrent issues in repo I worked on), and finally the files are veyr very small (400 kb together). Would it be fine to leave them here?
  • -the data generationr equires torch, which is not in the base docker image, how can I make the dataset use python-extras ? edit: solved it

Thanks in advance for the help!

@scottgigante-immunai

Copy link
Copy Markdown
Collaborator

@giovp I didn't realise how small they were. Yes, I think it's fine to host these on github. I will do a small refactor of the directory structure but otherwise this seems fine to me.

Comment thread openproblems/tasks/spatial_decomposition/datasets/destvi/generate.py Outdated
sc_anndata.uns["key_clustering"] = key_list
sc_anndata.uns["target_list"] = [1] + target_list

# write the full data

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove unused code

Comment thread openproblems/tasks/spatial_decomposition/datasets/destvi/generate.py Outdated
@giovp

giovp commented Aug 25, 2022

Copy link
Copy Markdown
Collaborator Author

hi @scottgigante-immunai ,

=================================== FAILURES ===================================
_ test_load_dataset_spatial_decomposition_destvi_True_openproblems_python_extras _
Traceback (most recent call last):
  File "/usr/local/lib/python3.8/site-packages/parameterized/parameterized.py", line 533, in standalone_func
    return func(*(a + p.args), **p.kwargs)
  File "<decorator-gen-55>", line 2, in test_load_dataset
  File "/__w/SingleCellOpenProblems/SingleCellOpenProblems/test/utils/docker.py", line 267, in docker_test
    run_image(image, f, timeout=timeout, retries=retries)
  File "/__w/SingleCellOpenProblems/SingleCellOpenProblems/test/utils/docker.py", line 223, in run_image
    return run.run(command, timeout=timeout)
  File "/__w/SingleCellOpenProblems/SingleCellOpenProblems/test/utils/run.py", line 116, in run
    _run_failed(p, error_raises, format_error)
  File "/__w/SingleCellOpenProblems/SingleCellOpenProblems/test/utils/run.py", line 39, in _run_failed
    raise error_raises(format_error(process))
AssertionError: docker exec 54d637e5e[427](https://github.com/giovp/SingleCellOpenProblems/runs/7940312715?check_suite_focus=true#step:12:428) /bin/bash /__w/SingleCellOpenProblems/SingleCellOpenProblems/test/docker_run.sh /__w/SingleCellOpenProblems/SingleCellOpenProblems/test/ /tmp/tmpftxboumk/test_load_dataset.py
Return code 1

DEBUG:openproblems:Loading destvi dataset
/__w/SingleCellOpenProblems/SingleCellOpenProblems/openproblems/tasks/spatial_decomposition/datasets/destvi/utils.py:123: FutureWarning: X.dtype being converted to np.float32 from int64. In the next version of anndata (0.9) conversion will not be automatic. Pass dtype explicitly to avoid this warning. Pass `AnnData(X, dtype=X.dtype, ...)` to get the future behavour.
  sc_anndata = anndata.AnnData(
/__w/SingleCellOpenProblems/SingleCellOpenProblems/openproblems/tasks/spatial_decomposition/datasets/destvi/utils.py:200: FutureWarning: X.dtype being converted to np.float32 from int64. In the next version of anndata (0.9) conversion will not be automatic. Pass dtype explicitly to avoid this warning. Pass `AnnData(X, dtype=X.dtype, ...)` to get the future behavour.
  st_anndata = anndata.AnnData(
DEBUG:openproblems:Loading destvi dataset
/__w/SingleCellOpenProblems/SingleCellOpenProblems/openproblems/tasks/spatial_decomposition/datasets/destvi/utils.py:123: FutureWarning: X.dtype being converted to np.float32 from int64. In the next version of anndata (0.9) conversion will not be automatic. Pass dtype explicitly to avoid this warning. Pass `AnnData(X, dtype=X.dtype, ...)` to get the future behavour.
  sc_anndata = anndata.AnnData(
/__w/SingleCellOpenProblems/SingleCellOpenProblems/openproblems/tasks/spatial_decomposition/datasets/destvi/utils.py:200: FutureWarning: X.dtype being converted to np.float32 from int64. In the next version of anndata (0.9) conversion will not be automatic. Pass dtype explicitly to avoid this warning. Pass `AnnData(X, dtype=X.dtype, ...)` to get the future behavour.
  st_anndata = anndata.AnnData(
Traceback (most recent call last):
  File "/tmp/tmpftxboumk/test_load_dataset.py", line 21, in <module>
    test_load_dataset(*('spatial_decomposition', 'destvi', True, '/tmp/tmpbubj56s1', 'openproblems-python-extras'), **{})
  File "/tmp/tmpftxboumk/test_load_dataset.py", line 13, in test_load_dataset
    assert adata2.uns["_from_cache"]
  File "/usr/local/lib/python3.8/site-packages/anndata/compat/_overloaded_dict.py", line 100, in __getitem__
    return self.data[key]
KeyError: '_from_cache'
DEBUG:openproblems:Removed data cache directory

the dataset addition gives this error, not sure how to fix this.

@scottgigante-immunai

Copy link
Copy Markdown
Collaborator

This comes from the fact that the dataset is not generated with an openproblems.data.utils.loader, which prevents caching and other nice features. We could move this over to /data but I doubt it would be useful to other tasks, since a) it's a simulation and b) it's very task-specific.

@giovp

giovp commented Sep 12, 2022

Copy link
Copy Markdown
Collaborator Author

This comes from the fact that the dataset is not generated with an openproblems.data.utils.loader, which prevents caching and other nice features. We could move this over to /data but I doubt it would be useful to other tasks, since a) it's a simulation and b) it's very task-specific.

thanks @scottgigante-immunai , catching up on this. How should I proceed then? Leave empty strings?

@scottgigante-immunai

Copy link
Copy Markdown
Collaborator

Definitely tag it with @loader, but leave the file where it is. For the reference URL you should cite the DestVI paper, and for the data URL you can reference wherever you get those raw files for the generation.

@scottgigante-immunai scottgigante-immunai left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tests are failing. Here's three reasons why (:

Comment thread openproblems/tasks/spatial_decomposition/datasets/destvi/utils.py
Comment thread openproblems/tasks/spatial_decomposition/datasets/destvi/utils.py Outdated
Comment thread openproblems/tasks/spatial_decomposition/datasets/destvi/utils.py Outdated
@giovp

giovp commented Oct 6, 2022

Copy link
Copy Markdown
Collaborator Author

@scottgigante-immunai still struggling with CI errors, could you give me some pointers? thanks a lot in advance

https://github.com/giovp/SingleCellOpenProblems/actions/runs/3169785696/jobs/5161931763

@scottgigante-immunai

Copy link
Copy Markdown
Collaborator

@scottgigante-immunai
scottgigante-immunai marked this pull request as ready for review October 12, 2022 15:33
@giovp

giovp commented Oct 12, 2022

Copy link
Copy Markdown
Collaborator Author

uou GA passed in fork CI!

@scottgigante-immunai scottgigante-immunai left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tests are passing! Woohoo!

@LuckyMD

LuckyMD commented Oct 12, 2022

Copy link
Copy Markdown
Collaborator

So now we find out if it's the dataset or the task somehow ^^. Looking forward to that full benchmarking run :D.

@codecov

codecov Bot commented Oct 12, 2022

Copy link
Copy Markdown

Codecov Report

Base: 94.71% // Head: 94.72% // Increases project coverage by +0.00% 🎉

Coverage data is based on head (40802ba) compared to base (ab4628a).
Patch coverage: 94.89% of modified lines in pull request are covered.

❗ Current head 40802ba differs from pull request most recent head d3213ac. Consider uploading reports for the commit d3213ac to get more accurate results

Additional details and impacted files
@@           Coverage Diff            @@
##             main     #526    +/-   ##
========================================
  Coverage   94.71%   94.72%            
========================================
  Files         140      142     +2     
  Lines        3483     3618   +135     
  Branches      176      188    +12     
========================================
+ Hits         3299     3427   +128     
- Misses        123      126     +3     
- Partials       61       65     +4     
Flag Coverage Δ
unittests 94.72% <94.89%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...sks/spatial_decomposition/datasets/destvi/utils.py 94.40% <94.40%> (ø)
.../spatial_decomposition/datasets/destvi/generate.py 100.00% <100.00%> (ø)
...s/tasks/spatial_decomposition/datasets/pancreas.py 100.00% <100.00%> (ø)
...lems/tasks/spatial_decomposition/datasets/utils.py 96.07% <100.00%> (ø)
openproblems/tasks/spatial_decomposition/utils.py 96.96% <100.00%> (+0.19%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@scottgigante-immunai
scottgigante-immunai merged commit 83b512e into openproblems-bio:main Oct 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants