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

Pixie percentile saving #1020

Merged
merged 19 commits into from
Jul 5, 2023
Merged

Pixie percentile saving #1020

merged 19 commits into from
Jul 5, 2023

Conversation

camisowers
Copy link
Contributor

@camisowers camisowers commented Jun 20, 2023

If you haven't already, please read through our contributing guidelines before opening your PR

What is the purpose of this PR?

  1. Closes In pixel clustering, if restarting create_pixel_matrix, not all FOVs are used to find the 99.9th percentile values #1016. Implement intermediate saving of per image 99.9th percent values.

In addition, two small corrections were made to the pixie code.

  1. adds a new channel_percentile_postnorm argument to create_pixel_matrix(), rather than hard coding 0.999.
  2. add fix for the edge case where an image has no positive pixels (previously would error out)
    (@HPiyadasa ran into this issue when trying to pixels cluster only SMA+ pixels within a mask)
  3. Should also close Data frame related warning generated in pixie cluster pixels notebook #900.

How did you implement your changes

  1. Add image values to quantile_data.csv as they are processed. Also check the FOVs included in the file upon a restart, on the off chance a FOV had a feather file written, but the quantile data failed to be saved. Such images will be re-processed.
  2. Set default channel_percentile_postnorm=0.999.
  3. If there are no pixel cluster labels in an image, simply return an empty array rather than trying to concatenate an empty list (this causes an error).

Remaining issues
Long term, we could probably prevent zero positive pixel images from attempting to be processed at all, but that likely requires a more in depth fix and the above solution works fine for now.

@camisowers camisowers added the bug Something isn't working label Jun 20, 2023
@camisowers camisowers self-assigned this Jun 20, 2023
@camisowers camisowers marked this pull request as ready for review June 21, 2023 21:38
@camisowers camisowers requested a review from cliu72 June 21, 2023 22:47
Copy link
Contributor

@alex-l-kong alex-l-kong left a comment

Choose a reason for hiding this comment

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

Comments about the loading process

src/ark/phenotyping/pixie_preprocessing.py Outdated Show resolved Hide resolved
src/ark/phenotyping/pixie_preprocessing.py Outdated Show resolved Hide resolved
src/ark/phenotyping/pixie_preprocessing.py Show resolved Hide resolved
src/ark/phenotyping/pixie_preprocessing.py Outdated Show resolved Hide resolved
src/ark/phenotyping/pixie_preprocessing.py Outdated Show resolved Hide resolved
src/ark/phenotyping/pixie_preprocessing.py Outdated Show resolved Hide resolved
Copy link
Contributor

@cliu72 cliu72 left a comment

Choose a reason for hiding this comment

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

Agree with Alex's comments, and added just a few inconsequential naming changes.

@camisowers
Copy link
Contributor Author

camisowers commented Jun 27, 2023

Consulted with @alex-l-kong and removed a failing test that was resulting from the missing channel normalization code. Will get added back in #985.

Copy link
Contributor

@cliu72 cliu72 left a comment

Choose a reason for hiding this comment

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

Looks good to me!

Copy link
Contributor

@alex-l-kong alex-l-kong left a comment

Choose a reason for hiding this comment

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

Looks good.

@ngreenwald ngreenwald merged commit 44b46c4 into main Jul 5, 2023
@ngreenwald ngreenwald deleted the pixie_percentile_saving branch July 5, 2023 17:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
4 participants