Skip to content
This repository has been archived by the owner on Jul 29, 2023. It is now read-only.

Inference bug, pandas docker version #153

Merged
merged 6 commits into from
Jun 17, 2022
Merged

Inference bug, pandas docker version #153

merged 6 commits into from
Jun 17, 2022

Conversation

JohannaRahm
Copy link
Contributor

Inference bug
Hi, I get an error when running inference with the latest version of the repo (master branch, #da7dcfc). The variable "channel_name" is automatically extracted from the meta_data in "inference->image_inference" and the "channel_name" info is used in the name of the saved images ("utils->aux_utils".)

The channel_name variable (str) is checked by the statement np.isnan(channel_name), which gives an error.

if np.isnan(channel_name):
    im_name += "_" + str(channel_name)

I changed the line to:

if channel_name is not None:
    im_name += "_" + str(channel_name)
  • no error when checking for channel_name value
  • if a channel name is defined, the name will be inserted in the name of the saved images

Command: python /home/pwrai/microDL/microDL/micro_dl/cli/inference_script.py --config /gpfs/CompMicro/projects/virtualstaining/2022_microDL_nuc_mem/configfiles/input_mem/config_inference_2022_03_15_mem_heavy_augmentation_test_40x_error.yml --gpu 3

Configfile (you can use this file to try out the inference): /gpfs/CompMicro/projects/virtualstaining/2022_microDL_nuc_mem/configfiles/input_mem/config_inference_2022_03_15_mem_heavy_augmentation_z12-74_test_40x.yml

MicroDL was executed in docker (image: microdl_sguo, name: microDL_JR) on hulk.

The inference branch was checked out from master #da7dcfc

Error:
image

Pandas version in docker
The pandas version must be set to 0.24.2, with 1.1.5 there are interferences with the numpy library. I downgraded the version in the requirements_docker.txt file to prevent the error.

Command: python /home/pwrai/microDL/microDL/micro_dl/cli/inference_script.py --config /gpfs/CompMicro/projects/virtualstaining/2022_microDL_nuc_mem/configfiles/input_mem/config_inference_2022_03_15_mem_heavy_augmentation_test_40x_error.yml --gpu 3

Error:
image

Copy link
Contributor

@jennyfolkesson jennyfolkesson left a comment

Choose a reason for hiding this comment

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

Thanks for catching and fixing this bug!

requirements_docker.txt Show resolved Hide resolved
@@ -198,7 +198,7 @@ def get_sms_im_name(time_idx=None,
"""

im_name = "img"
if np.isnan(channel_name):
if channel_name is not None:
Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you for catching this bug. The channel_name can actually be a NaN when loading a frames metadata csv file with pandas, so would it work to replace the line with:
if not pd.isnull(channel_name): ?
The reason this bug escaped notice is because there are no tests for get_sms_im_name in aux_utils_tests.py, so bonus points if you add a test. :-)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks! I replaced the line with if not pd.isnull(channel_name): and added a test. Curious about your feedback! :)

@codecov
Copy link

codecov bot commented Jun 16, 2022

Codecov Report

Merging #153 (79d714c) into master (da7dcfc) will increase coverage by 0.28%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #153      +/-   ##
==========================================
+ Coverage   76.39%   76.67%   +0.28%     
==========================================
  Files          53       53              
  Lines        4554     4554              
==========================================
+ Hits         3479     3492      +13     
+ Misses       1075     1062      -13     
Flag Coverage Δ
build 76.67% <100.00%> (+0.28%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
micro_dl/utils/aux_utils.py 94.84% <100.00%> (+5.57%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update da7dcfc...79d714c. Read the comment docs.

)
nose.tools.assert_equal(im_name, 'img_t00_p10.jpg')


Copy link
Contributor

Choose a reason for hiding this comment

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

🥇
Great job adding tests! My only comment is that I would separate these into two tests, which makes it easier to spot exactly where the code breaks if a test isn't passing. But that's optional.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done!

Co-authored-by: Jenny Folkesson <[email protected]>
@JohannaRahm JohannaRahm merged commit 7d5ab57 into master Jun 17, 2022
@JohannaRahm JohannaRahm deleted the inference branch June 17, 2022 08:17
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants