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

Adapt CI so it covers both new and old Macs, and installs required additional dependencies on M1 #408

Merged
merged 3 commits into from May 1, 2024
Merged

Adapt CI so it covers both new and old Macs, and installs required additional dependencies on M1 #408

merged 3 commits into from May 1, 2024

Conversation

alessandrofelder
Copy link
Member

@alessandrofelder alessandrofelder commented Apr 29, 2024 •
edited

Description

What is this PR

  • Bug fix
  • Addition of a new feature
  • Other

Why is this PR needed?

macos-latest is now a Silicon Mac runner, so we need to adapt CI accordingly

What does this PR do?

  • ensures CI is run on both Mac chip architectures
  • runs tensorflow include guard tests on Silicon mac instead.

References

Partial solution to brainglobe/BrainGlobe#63

How has this PR been tested?

CI that was breaking before, now passes

Is this a breaking change?

Now

Does this PR require an update to the documentation?

No

Checklist:

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

@alessandrofelder alessandrofelder marked this pull request as ready for review April 29, 2024 12:41
sfmig
sfmig approved these changes Apr 30, 2024
View reviewed changes
Copy link
Contributor

@sfmig sfmig left a comment

Choose a reason for hiding this comment

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

thanks @alessandrofelder, looks good too! 😁 Just a couple of questions out of curiosity.

@alessandrofelder alessandrofelder merged commit 6b529dc into main May 1, 2024
17 checks passed
@alessandrofelder alessandrofelder deleted the adapt-CI-to-silicon branch May 1, 2024 10:39
IgorTatarnikov added a commit that referenced this pull request May 10, 2024
@IgorTatarnikov @sfmig
@K-Meech @matham @pre-commit-ci @alessandrofelder @adamltyson
It/keras3 pytorch (#396)
5a0152b 
* 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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sfmig sfmig sfmig approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@alessandrofelder @sfmig