Nuclear fix #422
Nuclear fix #422
Conversation
|
Sorry for the delay, but this version should run much faster. I'm still verifying accuracy on the test dataset in |
|
Okay, I see the problem. For generating the figures for the Mesmer paper, I computed NC ratio the same way you now have it, outside of the loop and all at once. However, the first pass at the refactor to include these morphology metrics in ark computed it the same way the other stuff, cell-by-cell, leading to the slowdown. The next thing to check is why the wrong results are being returned, the indexing errors seems like the likely culprit. |
…or whole cell (and _feature_1 for nuclear)
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
alex-l-kong
requested review from
ngreenwald
and removed request for
awedwards and
ngreenwald
templates/Segment_Image_Data.ipynb
Outdated
| " delimiter='_feature_0',\n", | ||
| " force_ints=True)\n", | ||
| "\n", | ||
| "segmentation_labels_nuc = load_utils.load_imgs_from_dir(data_dir=deepcell_output_dir,\n", | ||
| " xr_dim_name='compartments',\n", | ||
| " xr_channel_names=['nuclear'],\n", | ||
| " files=[fov + '_feature_1.tif' for fov in fovs],\n", |
There was a problem hiding this comment.
We need to address the underlying issue with the load_imgs function, otherwise this is just going to happen again.
No error message was thrown when the delimiter didn't match the file name. Was it you or @ackagel who added the delimiter_optional flag to the extract_delimited_names function? What was the reasoning behind that?
There was a problem hiding this comment.
Sounds my (apparently not so handi)-work. I'll look into that, quickly rework the functionality, and rewrite/reimplement the test functions which should've caught this failure.
There was a problem hiding this comment.
@alex-l-kong I'll work on a PR for that and merge it into this branch once it's approved. Sounds good?
|
@awedwards for the time being, we've implemented a temporary fix on this branch addressing the |
* separates matching and trimming arguments for filename suffixes * adds test for matching file suffixes * updates `load_imgs_from_dir` usage in segmentation notebook * updates notebook test to include nuclear segmentations * changes filename matching from suffix based to substring based * corrects notebook test error and adds nuclear check for mibitiff notebook test * updates args in notebook tests * fixes typo in a sample tiff generating function * removes .DS_Store
* Initial commit of nuclear parameter fixing * Broadcast compute nc_ratio * Accidentally switched nuclear and whole cell * Add comments to new nc_ratio function * Add delimiter check to load_imgs_from_dir so both compartments don't get loaded * Add files parameter to load_imgs_from_dir so only _feature_0 loaded for whole cell (and _feature_1 for nuclear) * Add _feature_0 and _feature_1 file support for notebook * File delimiter fix (#424) * separates matching and trimming arguments for filename suffixes * adds test for matching file suffixes * updates `load_imgs_from_dir` usage in segmentation notebook * updates notebook test to include nuclear segmentations * changes filename matching from suffix based to substring based * corrects notebook test error and adds nuclear check for mibitiff notebook test * updates args in notebook tests * fixes typo in a sample tiff generating function * removes .DS_Store Co-authored-by: vacuous_planet <36260163+ackagel@users.noreply.github.com>
What is the purpose of this PR?
Closes #419.
nc_ratiois currently taking a very long time to compute. We address this bottleneck in this PR.How did you implement your changes
The original method worked well for small datasets but didn't scale well due to the inefficiencies of looping through hundreds to thousands of cells. To avoid the bottleneck of looping through each cell, we instead broadcast this operation.
Remaining issues
The actual accuracy of the values still needs to be checked. In addition to the test data provided in
ark, it would help to test on maybe a couple other datasets cause it looked like @awedwards was having issues of his own on his dataset.