Support single z-stack tif file for input #397

matham · 2024-04-10T04:53:13Z

Description

This is part of these other two PRs: brainglobe/brainglobe-workflows#88 and brainglobe/brainglobe-utils#67.

What is this PR

Bug fix
Addition of a new feature
Other

Why is this PR needed?

Without this, the pipeline can only read a folder of tiff files rather than a single tiff z-stack.

However, our computer has enough ram such that it is not a problem to load the full file to memory, so it'd be more efficient to do that. But without these changes I need to extract the stack to a sequence of tiff files.

What does this PR do?

Enables it so that it can read the tiff stack.

How has this PR been tested?

I used it with the pipeline to read our single file tiff stack.

Is this a breaking change?

Nope

Does this PR require an update to the documentation?

I'm not sure. I updated the function docs.

Checklist:

The code has been tested locally
Tests have been added to cover all new functionality (unit & integration)
The documentation has been updated to reflect any changes
The code has been formatted with pre-commit

Testing resulted in the following. I don't see anything here indicating it's because of my changes.

========================================================================================================== short test summary info =========================================================================================================== FAILED tests/core/test_integration/test_detection.py::test_detection_full[No free CPUs] - OSError: Unable to synchronously open file (truncated file: eof = 66060288, sblock->base_addr = 0, stored_eof = 184831728)
FAILED tests/core/test_integration/test_detection.py::test_detection_full[One CPU] - OSError: Unable to synchronously open file (truncated file: eof = 66060288, sblock->base_addr = 0, stored_eof = 184831728)
FAILED tests/core/test_integration/test_detection.py::test_callbacks - OSError: Unable to synchronously open file (truncated file: eof = 66060288, sblock->base_addr = 0, stored_eof = 184831728)
FAILED tests/core/test_integration/test_detection.py::test_synthetic_data - OSError: Unable to synchronously open file (truncated file: eof = 66060288, sblock->base_addr = 0, stored_eof = 184831728)
FAILED tests/core/test_integration/test_detection_structure_splitting.py::test_structure_splitting - OSError: Unable to synchronously open file (truncated file: eof = 66060288, sblock->base_addr = 0, stored_eof = 184831728)
====================================================================================== 5 failed, 94 passed, 2 xfailed, 6 warnings in 335.87s (0:05:35) ==========================================================

imagesc-bot · 2024-04-10T10:12:06Z

This pull request has been mentioned on Image.sc Forum. There might be relevant details there:

https://forum.image.sc/t/cellfinder-issue-indexerror-list-index-out-of-range/78192/3

adamltyson · 2024-04-18T13:50:52Z

@matham I know we have a loading function in this repo, but we probably shouldn't have. Would you have any issue with your new function (and our existing dask-based one) being moved into brainglobe_utils.imageio.load?

Tagging @brainglobe/active-devs to see if anyone else has any input.

K-Meech · 2024-04-19T09:01:55Z

I'd agree with @adamltyson - it'd be great if all the loading functions were brought together in brainglobe_utils.imageio.load. This could be added to the open PR on brainglobe-utils?

adamltyson · 2024-04-19T09:08:05Z

I'd agree with @adamltyson - it'd be great if all the loading functions were brought together in brainglobe_utils.imageio.load. This could be added to the open PR on brainglobe-utils?

We should probably do this though, not outsource fixing our technical debt to @matham! 😆

K-Meech

Looks good to me! I tried this out locally with a tiff file, and all worked great. I had one minor comment below.

Merging this will require brainglobe/brainglobe-utils#67 to be merged first

K-Meech · 2024-04-19T12:34:49Z

cellfinder/core/tools/IO.py

+    :return: The data as a dask/numpy array.
+    """
+    if path.endswith(".tiff") or path.endswith(".tif"):
+        tiff = TiffFile(path)


Could you use with TiffFile(path) as tiff: for this section? This will ensure the connection is closed correctly, as they require in their docs

Added change!

adamltyson · 2024-04-19T13:08:30Z

FYI I opened #405 to track moving the IO code

matham · 2024-04-22T05:54:53Z

And yeah it does make sense to me that the loading code should be in one place only in utils.

adamltyson · 2024-05-01T09:29:45Z

@K-Meech can this be merged now? We can tackle #405 at another time.

alessandrofelder · 2024-05-01T10:13:46Z

Yes, let's merge!

* replace tensorflow Tensor with keras tensor * add case for TF prep in prep_model_weights * add different backends to pyproject.toml * add backend configuration to cellfinder init file. tests passing with jax locally * define extra dependencies for cellfinder with different backends. run tox with TF backend * run tox using TF and JAX backend * install TF in brainmapper environment before running tests in CI * add backends check to cellfinder init file * clean up comments * fix tf-nightly import check * specify TF backend in include guard check * clarify comment * remove 'backend' from dependencies specifications * Apply suggestions from code review Co-authored-by: Igor Tatarnikov <61896994+IgorTatarnikov@users.noreply.github.com> * PyTorch runs utilizing multiple cores * PyTorch fix with default models * Tests run on every push for now * Run test on torch backend only * Fixed guard test to set torch as KERAS_BACKEND * KERAS_BACKEND env variable set directly in test_include_guard.yaml * Run test on python 3.11 * Remove tf-nightly from __init__ version check * Added 3.11 to legacy tox config * Changed legacy tox config for real this time * Don't set the wrong max_processing value * Torch is now set as the default backend * Tests only run with torch, updated comments * Unpinned torch version * Add codecov token (#403) * add codecov token * generate xml coverage report * add timeout to testing jobs * Allow turning off classification or detection in GUI (#402) * Allow turning off classification or detection in GUI. * Fix test. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Refactor to fix code analysis errors. * Ensure array is always 2d. * Apply suggestions from code review Co-authored-by: Igor Tatarnikov <61896994+IgorTatarnikov@users.noreply.github.com> --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Igor Tatarnikov <61896994+IgorTatarnikov@users.noreply.github.com> * Support single z-stack tif file for input (#397) * Support single z-stack tif file for input. * Fix commit hook. * Apply review suggestions. * Remove modular asv benchmarks (#406) * remove modular asv benchmarks * recover old structure * remove asv-specific lines from gitignore and manifest * prune benchmarks * Adapt CI so it covers both new and old Macs, and installs required additional dependencies on M1 (#408) * naive attempt at adapting to silicon mac CI * run include guard test on Silicon CI * double-check hdf5 is needed * Optimize cell detection (#398) (#407) * Replace coord map values with numba list/tuple for optim. * Switch to fortran layout for faster update of last dim. * Cache kernel. * jit ball filter. * Put z as first axis to speed z rolling (row-major memory). * Unroll recursion (no perf impact either way). * Parallelize cell cluster splitting. * Parallelize walking for full images. * Cleanup docs and pep8 etc. * Add pre-commit fixes. * Fix parallel always being selected and numba function 1st class warning. * Run hook. * Older python needs Union instead of |. * Accept review suggestion. * Address review changes. * num_threads must be an int. --------- Co-authored-by: Matt Einhorn <matt@einhorn.dev> * [pre-commit.ci] pre-commit autoupdate (#412) updates: - [github.com/pre-commit/pre-commit-hooks: v4.5.0 → v4.6.0](pre-commit/pre-commit-hooks@v4.5.0...v4.6.0) - [github.com/astral-sh/ruff-pre-commit: v0.3.5 → v0.4.3](astral-sh/ruff-pre-commit@v0.3.5...v0.4.3) - [github.com/psf/black: 24.3.0 → 24.4.2](psf/black@24.3.0...24.4.2) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> * Apply suggestions from code review Co-authored-by: sfmig <33267254+sfmig@users.noreply.github.com> * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Simplify model download (#414) * Simplify model download * Update model cache * Remove jax and tf tests * Standardise the data types for inputs to all be float32 * Force torch to use CPU on arm based macOS during tests * Added PYTORCH_MPS_HIGH_WATERMARK_RATION env variable * Set env variables in test setup * Try to set the default device to cpu in the test itself * Add device call to Conv3D to force cpu * Revert changes, request one cpu left free * Revers the numb cores, don't use arm based mac runner * Merged main, removed torch flags on cellfinder install for guards and brainmapper * Lowercase Torch * Change cache directory --------- Co-authored-by: sfmig <33267254+sfmig@users.noreply.github.com> Co-authored-by: Kimberly Meechan <24316371+K-Meech@users.noreply.github.com> Co-authored-by: Matt Einhorn <matt@einhorn.dev> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Alessandro Felder <alessandrofelder@users.noreply.github.com> Co-authored-by: Adam Tyson <code@adamltyson.com>

* remove pytest-lazy-fixture as dev dependency and skip test (with WG temp fix) * Test Keras is present (#374) * check if Keras present * change TF to Keras in CI * remove comment * change dependencies in pyproject.toml for Keras 3.0 * Migrate to Keras 3.0 with TF backend (#373) * remove pytest-lazy-fixture as dev dependency and skip test (with WG temp fix) * change tensorflow dependency for cellfinder * replace keras imports from tensorflow to just keras imports * add keras import and reorder * add keras and TF 2.16 to pyproject.toml * comment out TF version check for now * change checkpoint filename for compliance with keras 3. remove use_multiprocessing=False from fit() as it is no longer an input. test_train() passing * add multiprocessing parameters to cube generator constructor and remove from fit() signature (keras3 change) * apply temp garbage collector fix * skip troublesome test * skip running tests on CI on windows * remove commented out TF check * clean commented out code. Explicitly pass use_multiprocessing=False (as before) * remove str conversion before model.save * raise test_detection error for sonarcloud happy * skip running tests on windows on CI * remove filename comment and small edits * Replace TF references in comments and warning messages (#378) * change some old references to TF for the import check * change TF cached model to Keras * Cellfinder with Keras 3.0 and jax backend (#379) * replace tensorflow Tensor with keras tensor * add case for TF prep in prep_model_weights * add different backends to pyproject.toml * add backend configuration to cellfinder init file. tests passing with jax locally * define extra dependencies for cellfinder with different backends. run tox with TF backend * run tox using TF and JAX backend * install TF in brainmapper environment before running tests in CI * add backends check to cellfinder init file * clean up comments * fix tf-nightly import check * specify TF backend in include guard check * clarify comment * remove 'backend' from dependencies specifications * Apply suggestions from code review Co-authored-by: Igor Tatarnikov <61896994+IgorTatarnikov@users.noreply.github.com> --------- Co-authored-by: Igor Tatarnikov <61896994+IgorTatarnikov@users.noreply.github.com> * Run cellfinder with JAX in Windows tests in CI (#382) * use jax backend in brainmapper tests in CI * skip TF backend on windows * fix pip install cellfinder for brainmapper CI tests * add keras env variable for brainmapper CLI tests * fix prep_model_weights * It/keras3 pytorch (#396) * replace tensorflow Tensor with keras tensor * add case for TF prep in prep_model_weights * add different backends to pyproject.toml * add backend configuration to cellfinder init file. tests passing with jax locally * define extra dependencies for cellfinder with different backends. run tox with TF backend * run tox using TF and JAX backend * install TF in brainmapper environment before running tests in CI * add backends check to cellfinder init file * clean up comments * fix tf-nightly import check * specify TF backend in include guard check * clarify comment * remove 'backend' from dependencies specifications * Apply suggestions from code review Co-authored-by: Igor Tatarnikov <61896994+IgorTatarnikov@users.noreply.github.com> * PyTorch runs utilizing multiple cores * PyTorch fix with default models * Tests run on every push for now * Run test on torch backend only * Fixed guard test to set torch as KERAS_BACKEND * KERAS_BACKEND env variable set directly in test_include_guard.yaml * Run test on python 3.11 * Remove tf-nightly from __init__ version check * Added 3.11 to legacy tox config * Changed legacy tox config for real this time * Don't set the wrong max_processing value * Torch is now set as the default backend * Tests only run with torch, updated comments * Unpinned torch version * Add codecov token (#403) * add codecov token * generate xml coverage report * add timeout to testing jobs * Allow turning off classification or detection in GUI (#402) * Allow turning off classification or detection in GUI. * Fix test. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Refactor to fix code analysis errors. * Ensure array is always 2d. * Apply suggestions from code review Co-authored-by: Igor Tatarnikov <61896994+IgorTatarnikov@users.noreply.github.com> --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Igor Tatarnikov <61896994+IgorTatarnikov@users.noreply.github.com> * Support single z-stack tif file for input (#397) * Support single z-stack tif file for input. * Fix commit hook. * Apply review suggestions. * Remove modular asv benchmarks (#406) * remove modular asv benchmarks * recover old structure * remove asv-specific lines from gitignore and manifest * prune benchmarks * Adapt CI so it covers both new and old Macs, and installs required additional dependencies on M1 (#408) * naive attempt at adapting to silicon mac CI * run include guard test on Silicon CI * double-check hdf5 is needed * Optimize cell detection (#398) (#407) * Replace coord map values with numba list/tuple for optim. * Switch to fortran layout for faster update of last dim. * Cache kernel. * jit ball filter. * Put z as first axis to speed z rolling (row-major memory). * Unroll recursion (no perf impact either way). * Parallelize cell cluster splitting. * Parallelize walking for full images. * Cleanup docs and pep8 etc. * Add pre-commit fixes. * Fix parallel always being selected and numba function 1st class warning. * Run hook. * Older python needs Union instead of |. * Accept review suggestion. * Address review changes. * num_threads must be an int. --------- Co-authored-by: Matt Einhorn <matt@einhorn.dev> * [pre-commit.ci] pre-commit autoupdate (#412) updates: - [github.com/pre-commit/pre-commit-hooks: v4.5.0 → v4.6.0](pre-commit/pre-commit-hooks@v4.5.0...v4.6.0) - [github.com/astral-sh/ruff-pre-commit: v0.3.5 → v0.4.3](astral-sh/ruff-pre-commit@v0.3.5...v0.4.3) - [github.com/psf/black: 24.3.0 → 24.4.2](psf/black@24.3.0...24.4.2) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> * Apply suggestions from code review Co-authored-by: sfmig <33267254+sfmig@users.noreply.github.com> * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Simplify model download (#414) * Simplify model download * Update model cache * Remove jax and tf tests * Standardise the data types for inputs to all be float32 * Force torch to use CPU on arm based macOS during tests * Added PYTORCH_MPS_HIGH_WATERMARK_RATION env variable * Set env variables in test setup * Try to set the default device to cpu in the test itself * Add device call to Conv3D to force cpu * Revert changes, request one cpu left free * Revers the numb cores, don't use arm based mac runner * Merged main, removed torch flags on cellfinder install for guards and brainmapper * Lowercase Torch * Change cache directory --------- Co-authored-by: sfmig <33267254+sfmig@users.noreply.github.com> Co-authored-by: Kimberly Meechan <24316371+K-Meech@users.noreply.github.com> Co-authored-by: Matt Einhorn <matt@einhorn.dev> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Alessandro Felder <alessandrofelder@users.noreply.github.com> Co-authored-by: Adam Tyson <code@adamltyson.com> * Set pooling padding to valid by default on all MaxPooling3D layers * Removed tf error suppression and other tf related functions * Force torch to use cpu device when CELLFINDER_TEST_DEVICE env variable set to cpu * Added nev variable to test step * Use the GITHUB ACTIONS environemntal variable instead * Added docstring for fixture setting device to cpu on arm based mac * Revert changes to no_free_cpus being fixture, and default param * Fixed typo in test_and_deploy.yml * Set multiprocessing to false for the data generators * Update all cache steps to match * Remove reference to TF * Make sure tests can run locally when GITHUB_ACTIONS env variable is missing2 * Removed warning when backend is not configured * Set the label tensor to be float32 to ensure compatibility with mps * Always set KERAS_BACKEND to torch on init * Remove code in __init__ checking for if backend is installed --------- Co-authored-by: sfmig <33267254+sfmig@users.noreply.github.com> Co-authored-by: Kimberly Meechan <24316371+K-Meech@users.noreply.github.com> Co-authored-by: Matt Einhorn <matt@einhorn.dev> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Alessandro Felder <alessandrofelder@users.noreply.github.com> Co-authored-by: Adam Tyson <code@adamltyson.com>

matham added 2 commits April 9, 2024 23:41

Support single z-stack tif file for input.

86b5b71

Fix commit hook.

97b8bd7

This was referenced Apr 10, 2024

Support single z-stack tif file for input brainglobe/brainglobe-workflows#88

Merged

Support single z-stack tif file for input brainglobe/brainglobe-utils#67

Merged

adamltyson requested a review from a team April 10, 2024 10:15

K-Meech approved these changes Apr 19, 2024

View reviewed changes

matham added 2 commits April 22, 2024 01:47

Merge branch 'main' of github.com:matham/cellfinder into io

c083b5e

Apply review suggestions.

ab033e1

alessandrofelder merged commit b1b285c into brainglobe:main May 1, 2024
14 of 16 checks passed

matham deleted the io branch May 1, 2024 17:04

adamltyson mentioned this pull request May 9, 2024

cellfinder v1.3.0 blog post brainglobe/brainglobe.github.io#177

Closed

alessandrofelder mentioned this pull request May 15, 2024

Add image IO loading code from cellfinder & create image IO submodule within IO brainglobe/brainglobe-utils#73

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support single z-stack tif file for input #397

Support single z-stack tif file for input #397

matham commented Apr 10, 2024 •

edited

imagesc-bot commented Apr 10, 2024

adamltyson commented Apr 18, 2024

K-Meech commented Apr 19, 2024

adamltyson commented Apr 19, 2024

K-Meech left a comment

K-Meech Apr 19, 2024

matham Apr 22, 2024

adamltyson commented Apr 19, 2024

matham commented Apr 22, 2024

adamltyson commented May 1, 2024

alessandrofelder commented May 1, 2024

Support single z-stack tif file for input #397

Support single z-stack tif file for input #397

Conversation

matham commented Apr 10, 2024 • edited

Description

How has this PR been tested?

Is this a breaking change?

Does this PR require an update to the documentation?

Checklist:

imagesc-bot commented Apr 10, 2024

adamltyson commented Apr 18, 2024

K-Meech commented Apr 19, 2024

adamltyson commented Apr 19, 2024

K-Meech left a comment

Choose a reason for hiding this comment

K-Meech Apr 19, 2024

Choose a reason for hiding this comment

matham Apr 22, 2024

Choose a reason for hiding this comment

adamltyson commented Apr 19, 2024

matham commented Apr 22, 2024

adamltyson commented May 1, 2024

alessandrofelder commented May 1, 2024

matham commented Apr 10, 2024 •

edited