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.

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

Reduce memory overhead and improve performance of normalize_colors_pca #328

Merged

Conversation

grlee77
Copy link
Contributor

@grlee77 grlee77 commented Jul 5, 2022

In this PR we change the following line:

conc_norm = conc_raw * normalization_factors[:, cp.newaxis]

to instead modify conc_raw in-place. The conc_raw array is an intermediate array of concentrations, so changing to an in-place operation will not effect user-facing code. The benefit is avoiding an extra copy of the concentrations array. It also avoids an unintended promotion to float64 due to multiplication by a normalization_factors array that was potentially higher precision than conc_raw.

For a fairly large uint8 input array of shape (26420, 19920, 3), the conc_raw array ends up as an ~4GB float32 array. Without the change in this PR there will be a new 8GB float64 conc_norm array created. By avoiding this we substantially reduce memory use.

We also make sure the ref_max_conc and ref_stain_coeff inputs to _normalized_from_concentration match the precision of the image. Due to this, a 25% improvement in performance was observed (due to float32 rather than float64 computations in the remainder of the _normalized_from_concentrations function).

We try to avoid in-place modifications of inputs in user-facing functions, but
for this private, internal function it is needed in order to reduce memory
overhead due to making a copy of the potentially large  array.

The change here also avoids unintentional promotion from float32 to float64
that was previously occuring due to multiplication of float32  with
float64 . The in-place multiplication avoids this
potential promotion in precision, preserving memory and improving performance
of the subsequent  call.
@grlee77 grlee77 added improvement Improves an existing functionality non-breaking Introduces a non-breaking change performance Performance improvement labels Jul 5, 2022
@grlee77 grlee77 requested a review from a team as a code owner July 5, 2022 19:21
Copy link
Member

@jakirkham jakirkham left a comment

Choose a reason for hiding this comment

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

Thanks Greg! 🙏

@jakirkham
Copy link
Member

@gpucibot merge

@rapids-bot rapids-bot bot merged commit c3e5a9a into rapidsai:branch-22.08 Jul 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improvement Improves an existing functionality non-breaking Introduces a non-breaking change performance Performance improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants