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

[RELEASE] cucim v24.06 #735

Merged
merged 16 commits into from
Jun 5, 2024
Merged

[RELEASE] cucim v24.06 #735

merged 16 commits into from
Jun 5, 2024

Conversation

raydouglass
Copy link
Member

❄️ Code freeze for branch-24.06 and v24.06 release

What does this mean?

Only critical/hotfix level issues should be merged into branch-24.06 until release (merging of this PR).

What is the purpose of this PR?

  • Update documentation
  • Allow testing for the new release
  • Enable a means to merge branch-24.06 into main for the release

raydouglass and others added 15 commits March 15, 2024 10:46
@raydouglass
DOC v24.06 Updates [skip ci]
4555be5
@GPUtester
Merge pull request #715 from rapidsai/branch-24.04
5ff4879 
Forward-merge branch-24.04 into branch-24.06 [skip ci]
@GPUtester
Merge pull request #718 from rapidsai/branch-24.04
4f1cec2 
Forward-merge branch-24.04 into branch-24.06
@GPUtester
Merge pull request #720 from rapidsai/branch-24.04
963f577 
Forward-merge branch-24.04 into branch-24.06
@galipremsagar
Enable all tests for arm jobs (#717)
66f072f 
This PR enables running all tests in `arm` jobs

Authors:
  - GALI PREM SAGAR (https://github.com/galipremsagar)
  - https://github.com/jakirkham

Approvers:
  - Ray Douglass (https://github.com/raydouglass)
  - https://github.com/jakirkham

URL: #717
@GPUtester
Merge pull request #723 from rapidsai/branch-24.04
160c5e6 
Forward-merge branch-24.04 into branch-24.06
@AyodeAwe
prevent path conflict (#713)
686dba6 
This will make builds fail when there are path conflicts

Authors:
  - Jake Awe (https://github.com/AyodeAwe)
  - https://github.com/jakirkham

Approvers:
  - Joseph (https://github.com/jolorunyomi)

URL: #713
@grlee77
removal of deprecated functions/kwargs scheduled for release 24.06 (#724
16d015b 
)

This MR completes a few scheduled deprecations
- removes renamed `get_xyz_coords` function (it already wasn't accessible from the top-level `cucim.skimage.color` namespace)
- removes deprecated `return_error` kwarg from `phase_cross_correlation` (error is always returned)
- removes deprecated `random_state` kwarg from `medial_axis` (it was renamed to `rng` previously)

Authors:
  - Gregory Lee (https://github.com/grlee77)

Approvers:
  - Gigon Bae (https://github.com/gigony)

URL: #724
@grlee77
Remove code needed to support Python < 3.9 and apply ruff's pyupgrade…
3e67c2f 
… rules (#725)

`math.prod` was added in Python 3.8
`math.lcm` was added in Python 3.9

`pyproject.toml` was updated to have `ruff` also check [pyupgrade rules](https://docs.astral.sh/ruff/rules/#pyupgrade-up)

Authors:
  - Gregory Lee (https://github.com/grlee77)

Approvers:
  - https://github.com/jakirkham

URL: #725
@gigony
Update the test criteria for test_read_random_region_cpu_memleak (#726)
edd1da3 
We are seeing intermittent errors in CI/CD

See #725 (comment)

>Seeing one test failure in [this CI job]( https://github.com/rapidsai/cucim/actions/runs/8716002781/job/23909243290?pr=725#step:8:495 ):

```python
_ test_read_random_region_cpu_memleak[testimg_tiff_stripe_4096x4096_256_deflate] _
[gw6] linux -- Python 3.9.19 /pyenv/versions/3.9.19/bin/python
python/cucim/tests/performance/clara/test_read_region_memory_usage.py:100: in test_read_random_region_cpu_memleak
    assert mem_usage_history[-1] - mem_usage_history[1] < iteration * 100
E   assert (828035072 - 826982400) < (1000 * 100)
```

I couldn't determine which part of the code is increasing memory usage, but it's clear that there is no memory leak on the C++ side, as memory usage does not continuously increase per iteration.

This patch updates the validation logic to improve reliability:
- Increase the number of iterations by 10 times.
- Ensure that the memory leak per iteration does not exceed 768 bytes.
- Verify whether the increase in memory usage is continuous.

Authors:
  - Gigon Bae (https://github.com/gigony)

Approvers:
  - Gregory Lee (https://github.com/grlee77)
  - https://github.com/jakirkham

URL: #726
@grlee77
Use SciPy's KDTree instead of deprecated cKDTree (#733)
fb94e16 
Functionality is equivalent since SciPy 1.6 (see note [here](https://docs.scipy.org/doc/scipy-1.13.0/reference/generated/scipy.spatial.cKDTree.html)).

I pinned SciPy to >= 1.6 (released Dec 31, 2020)

This MR also adds some missing test cases for the `_ensure_spacing` helper function used by `cucim.skimage.feature.peak_local_max`. The new tests revealed a bug in that function in the case of non-scalar `spacing` which is now fixed.

Also, CuPy recently added KDTree so we can hopefully improve performance by moving to that in the future. I opened issue #732 as a reminder to investigate that.

Authors:
  - Gregory Lee (https://github.com/grlee77)

Approvers:
  - Ray Douglass (https://github.com/raydouglass)
  - https://github.com/jakirkham

URL: #733
@hcho3
Migrate to {{ stdlib("c") }} (#731)
6c467c6 
The `sysroot*` syntax is getting phased out (conda-forge/conda-forge.github.io#2102).
The recommendation is to move to `{{ stdlib("c") }}`.

Ref rapidsai/build-planning#39

Authors:
  - Philip Hyunsu Cho (https://github.com/hcho3)

Approvers:
  - Bradley Dice (https://github.com/bdice)
  - https://github.com/jakirkham
  - Ray Douglass (https://github.com/raydouglass)

URL: #731
@mroeschke
Enable FutureWarnings/DeprecationWarnings as errors (#734)
633aa35 
As part of rapidsai/build-planning#26, enabling Python test failures for `FutureWarning`s and `DeprecationWarning`s

Authors:
  - Matthew Roeschke (https://github.com/mroeschke)
  - Gigon Bae (https://github.com/gigony)

Approvers:
  - https://github.com/jakirkham

URL: #734
@grlee77
Implement upstream changes from scikit-image 0.23 (part 1 of 2) (#727)
0bd603a 
This MR updates cuCIM for consistency with version scikit-image `0.23.2rc1`. There were a relatively large number of changes in the `morphology` module, so I am going to submit those in a subsequent MR. I will also bump the scikit-image version pinning as part of that second MR.

The commits in this MR can be reviewed individually. Much of the new code is private functions in `cucim.skimage._shared` that are copied from upstream and don't need to be reviewed in detail.

The highlights are:

- `cucim.skimage.measure.regionprops` (and `regionprops_table`) support one new region property: `intensity_std` 
- `cucim.skimage.segmentation.expand_labels` now supports a `spacing` keyword argument to take a pixel's physical dimensions into account.
- additional modules use `__init__.pyi` instead of just `__init__.py`
- various fixes to documentation strings (consistent shape notation, etc.)
- adopted the private `_shared/compat.py` for future NumPy 2.0/SciPy compatibility
- adopted private test utility `assert_stacklevel`
- updated the private utility functions for handling deprecations to match those upstream
  - update existing decorators to the new ones
  - new rename of output->out for `cucim.skimage.filters.gaussian`

Authors:
  - Gregory Lee (https://github.com/grlee77)

Approvers:
  - Gigon Bae (https://github.com/gigony)

URL: #727
@grlee77
Implement upstream changes from scikit-image 0.23 (part 2 of 2: morph…
b23da22 
…ology) (#728)

There were a number of non-trivial changes to the morphology module, so I broke those out from the other changes in #727. Please review and merge that MR first before reviewing this one.

Highlights from upstream are:
- binary morphology functions have a new `mode` argument that controls how values outside the image boundaries are interpreted
- grayscale morphology functions have new `mode` and `cval` arguments that control how boundaries are extended (these were already available in `scipy.ndimage`/`cupyx.scipy.ndimage`, they just weren't exposed via the `skimage`/`cuCIM` APIs)
- binary and grayscale morphology functions have bug fixes in the case of even-sized/non-symmetric footprints
  - additional corresponding test cases were added

Aside from the upstream changes, novel changes in this MR are:
- refactored utility functions to mirror and pad the footprints to allow use with the cuCIM-specific optimization of passing a tuple for rectangular footprints instead of explicitly allocating a GPU footprint array
- refactored some test cases to better use `pytest.mark.parametrize`
- some grayscale tests now compare directly to `skimage` CPU outputs instead fetching previously saved values
- bumped our version pinning for scikit-image to allow 0.23.x to be installed

I marked as "non-breaking" as the existing behavior has not changed except in the case of the bug fixes for even-sized footprints.

Authors:
  - Gregory Lee (https://github.com/grlee77)

Approvers:
  - Gigon Bae (https://github.com/gigony)
  - Jake Awe (https://github.com/AyodeAwe)

URL: #728
@raydouglass raydouglass requested review from a team as code owners May 31, 2024 13:18
@raydouglass raydouglass merged commit c380fde into main Jun 5, 2024
2 of 3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@raydouglass @GPUtester @galipremsagar @AyodeAwe @grlee77 @gigony @hcho3 @mroeschke