Add codecov token #403

K-Meech · 2024-04-16T10:01:50Z

Description

What is this PR

Bug fix
Addition of a new feature
Other

Why is this PR needed?
Codecov is currently complaining on all actions runs with: secret-codecov-token is not set. This will be required in the future. See https://docs.codecov.com/docs/quick-start#step-2-get-the-repository-upload-token on how to get a token. and Codecov: Failed to properly upload report: The process 'D:\a\_actions\codecov\codecov-action\v4\dist\codecov.exe' failed with exit code 1

What does this PR do?
This PR ensures test_and_deploy is using the codecov token, as recommended in the readme for the neuroinformatics unit test action

References

None

How has this PR been tested?

Hopefully codecov won't error on the github actions runs for this PR

Is this a breaking change?

No

Does this PR require an update to the documentation?

No

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

K-Meech · 2024-04-16T12:58:27Z

Adding the token fixed the first warning, but not the second: Codecov: Failed to properly upload report: The process 'D:\a\_actions\codecov\codecov-action\v4\dist\codecov.exe' failed with exit code 1. Looking in the logs, it seems this is because it can't find any coverage reports to upload. I've updated the tox settings in pyproject.toml to create an xml coverage report, so hopefully that fixes the issue. I'll wait and see what the new github actions run looks like.

K-Meech · 2024-04-16T14:37:01Z

That seems to fix codecov! No more codecov complaints on the latest actions run. The run tests with numba disabled action is hanging though, due to a segmentation fault. Not sure what is causing this - it seems unrelated to any changes in this PR.

IgorTatarnikov · 2024-04-16T14:43:12Z

The run tests with numba disabled segfault seems to be sporadic. I ran into the same issue here but re-running it made it happy. Not sure what's going on!

willGraham01 · 2024-04-17T08:03:03Z

The run tests with numba disabled segfault seems to be sporadic. I ran into the same issue here but re-running it made it happy. Not sure what's going on!

My only guess off the top of my head would be that the runs got runners with different specs (maybe an older runner vs a newer one)? But looking at the attempts Igor linked to, it seems it's the same machine (specs-wise) so that rules that out.

Can we replicate this seg-fault locally? Or is it just a CI thing?

IgorTatarnikov · 2024-04-17T08:40:53Z

... Seems I marked my previous as duplicate and can't undo it. Reposting for posterity's sake.

I tried running the tests locally on Ubuntu 22.04 having set NUMBA_DISABLE_JIT=1 but I couldn't replicate the seg fault over 5 runs. I first ran all tests, then focused on running just test_detection.py to save time. Seems it might be a CI thing?

K-Meech · 2024-04-17T08:55:03Z

Also, it would be good to set timeout_minutes on the test_numba_disabled action, as is done for the standard test. I see the last run went for nearly 6 hours before it was cancelled!

willGraham01 · 2024-04-18T09:12:54Z

A brief glance at the logs indicates that the seg-fault is thrown during garbage collection whilst in a multiprocessing thread.

Python's garbage collector is not deterministic, which is the main motivator behind my following guesses 😅

One thread is finishing / exiting earlier than the others, and cleans up some (implicitly) shared memory before the other threads can finish? This is also supported by Kimberly's discovery that the most recent test hung for 6hrs then was killed - we might end be ending up in a deadlock caused by something like this.
We're not respecting the private/shared memory for each thread. Don't know if Python multiprocessing cares about these concepts in the same way that something like C++ does, though.
Some combination of the above combined with how GitHub runners handle threading.

But I could be well off the money in each of those though.

adamltyson · 2024-04-18T09:14:42Z

Can this be reproduced locally be limiting the number of CPUs cellfinder can use? This may explain why it only happens on the relatively low-spec GitHub actions runners.

willGraham01 · 2024-04-18T09:34:54Z

Just going to open #404 to move the seg-fault conversation there, so we can merge this PR and separate out the issue.

@K-Meech feel free to merge if everything you intended for the PR is ready!

* 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>

add codecov token

c3ceab5

K-Meech requested a review from a team April 16, 2024 10:01

willGraham01 approved these changes Apr 16, 2024

View reviewed changes

generate xml coverage report

ab261f2

This comment was marked as duplicate.

Sign in to view

add timeout to testing jobs

b91a36a

willGraham01 mentioned this pull request Apr 18, 2024

[BUG] Intermittent Segmentation Faults on CI #404

Closed

K-Meech merged commit 560909f into main Apr 18, 2024
16 checks passed

K-Meech deleted the codecov_token branch April 18, 2024 09:49

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add codecov token #403

Add codecov token #403

K-Meech commented Apr 16, 2024

K-Meech commented Apr 16, 2024

K-Meech commented Apr 16, 2024

IgorTatarnikov commented Apr 16, 2024

willGraham01 commented Apr 17, 2024 •

edited

This comment was marked as duplicate.

IgorTatarnikov commented Apr 17, 2024 •

edited

K-Meech commented Apr 17, 2024

willGraham01 commented Apr 18, 2024 •

edited

adamltyson commented Apr 18, 2024

willGraham01 commented Apr 18, 2024

Add codecov token #403

Add codecov token #403

Conversation

K-Meech commented Apr 16, 2024

Description

References

How has this PR been tested?

Is this a breaking change?

Does this PR require an update to the documentation?

Checklist:

K-Meech commented Apr 16, 2024

K-Meech commented Apr 16, 2024

IgorTatarnikov commented Apr 16, 2024

willGraham01 commented Apr 17, 2024 • edited

This comment was marked as duplicate.

IgorTatarnikov commented Apr 17, 2024 • edited

K-Meech commented Apr 17, 2024

willGraham01 commented Apr 18, 2024 • edited

adamltyson commented Apr 18, 2024

willGraham01 commented Apr 18, 2024

willGraham01 commented Apr 17, 2024 •

edited

IgorTatarnikov commented Apr 17, 2024 •

edited

willGraham01 commented Apr 18, 2024 •

edited