Skip to content

Update Transforms to _Also_ Support Non-Square Images#1

Open
williamyolland wants to merge 1 commit into
ISIC-Research:mainfrom
williamyolland:update-transforms-to-support-nonsquare
Open

Update Transforms to _Also_ Support Non-Square Images#1
williamyolland wants to merge 1 commit into
ISIC-Research:mainfrom
williamyolland:update-transforms-to-support-nonsquare

Conversation

@williamyolland

Copy link
Copy Markdown

Swap Resize for SmallestMaxSize + CenterCrop to support non-square images as input in addition to square images.

  • For images which are already square, this combination performs the same operation as a Resize.
  • For images which are non-square, this combination will resize the image such that the smallest of the two sides is image_size, before center cropping the image to image_size x image_size.

This behavior aligns with the publication (Supplementary Material: Data Augmentation), and allows users to run predictions over the PROVE-AI data downloaded from ISIC Archive without requiring an additional step of center cropping all images before running the code.

…ages as input _in addition to_ square images
@neuroelf

Copy link
Copy Markdown
Contributor

Hello William, thanks so much for engaging! Can you please confirm that you are having a problem with running the inference using the code in "predict.py"? The reason I am asking: lines 902 - 907 in that file are

                        if image_w > image_h:
                            to_crop = (image_w - image_h) // 2
                            image_data = image_data.crop((to_crop, 0, to_crop + image_h, image_h))
                        elif image_h > image_w:
                            to_crop = (image_h - image_w) // 2
                            image_data = image_data.crop((0, to_crop, image_w, to_crop + image_w))

meaning that, at least with the algorithm as implemented, only the central square portion of any lesion image will be considered (rather than squishing the image into a square shape to run through the network for inference).

I believe this should work for pretty much all images...

@williamyolland

Copy link
Copy Markdown
Author

Hello Jochen, thanks for the speedy reply. Yes, this code would also solve the problem, but for my needs this code was never reached due to the if-statement on line 894. Perhaps I'm running the code slightly out of scope, which is introducing this issue.

My use-case was to bring in a new folder of images from elsewhere, separate from the jpeg-melanoma-size-x-size image collections that you had provided. I formatted my CSV to match the expected columns based on the jpeg-melanoma-size-x-size data sets, but all of the images in my test set already exist so this code was not reached. I think I understand now that this code is meant to duplicate the images back to disk at different sizes to speed up inference. I suppose I set this up incorrectly on my side and this code was never reached.

I was able to reproduce the paper results by making my change to the validation transformations, but I think that if I had spent a bit more time on data setup then I would have reached this referenced code section which would have also worked. Feel free to disregard my suggested change, but perhaps you could add a line or two to the README to make it a little more clear exactly how a user is expected to set up the data directories to avoid any confusion like mine in the future.

Thank you very much 🙏

@neuroelf

Copy link
Copy Markdown
Contributor

Hey William, thanks for getting back! If the code changes solve an actual use case (that I hadn't considered), I'd rather merge the PR. I just want to make sure there isn't any (big) cost and/or side-effect associated with it. I'll try this out over the next couple days, and if I have any more questions, I'll be in touch!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants