Improve the auto_size logic during coordinates generation#11
Improve the auto_size logic during coordinates generation#11Dootmaan wants to merge 1 commit intobirkhoffkiki:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR addresses inconsistencies in the auto-sizing functionality for patch coordinate generation between single-process and multi-process execution modes in create_patches_fp.py. It adds a user-controllable --auto_size parameter to enable/disable automatic patch size adjustment based on WSI magnification.
Changes:
- Added
--auto_sizecommand-line argument (default: False) for explicit control over patch size adjustment - Modified
seg_and_patch()to conditionally apply auto-sizing only when enabled, fixing forced auto-sizing behavior - Modified
mp_seg_and_patch()to support auto-sizing, fixing previously disabled functionality in multi-process mode - Updated README.md with documentation about the auto_size option and usage notes
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| create_patches_fp.py | Added auto_size parameter to both patching functions, implemented conditional auto-sizing logic, and integrated the parameter into argument parsing and function calls |
| README.md | Added documentation notes explaining the auto_size functionality, its purpose, and usage recommendations for maintaining consistency between coordinate generation and image cropping |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| else: | ||
| fn = seg_and_patch | ||
| seg_times, patch_times = fn(**directories, **parameters, patch_size=args.patch_size, step_size=args.step_size, seg=args.seg, use_default_params=False, save_mask=True, stitch=args.stitch, patch_level=args.patch_level, patch=args.patch, process_list=process_list, auto_skip=args.no_auto_skip, wsi_format=wsi_format) | ||
| seg_times, patch_times = fn(**directories, **parameters, patch_size=args.patch_size, step_size=args.step_size, seg=args.seg, use_default_params=False, save_mask=True, stitch=args.stitch, patch_level=args.patch_level, patch=args.patch, process_list=process_list, auto_skip=args.no_auto_skip, auto_size=args.auto_size, wsi_format=wsi_format) |
There was a problem hiding this comment.
When use_mp is True, mp_seg_and_patch is called but it does not return any values. The function is missing a return statement (similar to seg_and_patch which returns seg_times and patch_times at line 309). This will cause a TypeError when trying to unpack None into seg_times and patch_times. The mp_seg_and_patch function needs to be updated to return timing values.
| | OmiCLIP | `omiclip` | [Github](https://github.com/GuangyuWangLab2021/Loki) | | ||
| | PathoCLIP | `pathoclip` | [Github](https://github.com/wenchuan-zhang/patho-r1) | | ||
| --- | ||
| ------------------------- |
There was a problem hiding this comment.
The table separator has inconsistent formatting. The row contains only dashes with no pipe characters, which breaks the markdown table structure. It should be removed or properly formatted as a standard table row with pipe delimiters.
| ------------------------- |
This PR fixes a workflow bug in create_patches_fp.py where the auto_size functionality behaved inconsistently between single-process and multi-process execution modes, and adds a user-controllable parameter to enable/disable automatic patch size adjustment.
Problem Description
The original implementation had the following issues:
Forced auto-sizing in single-process mode: The seg_and_patch() function always called adjust_size() to automatically adjust patch_size and step_size based on WSI magnification (mpp), regardless of user preference.
Disabled auto-sizing in multi-process mode: The mp_seg_and_patch() function never called adjust_size(), always using the user-specified fixed values for patch_size and step_size.
No user control: There was no command-line parameter to control this behavior, making it impossible for users to choose between fixed-size and auto-adjusted patching strategies. In contrast, this function can be turn on/off for extract_images.py easily.
Inconsistent behavior: This resulted in different processing results when running the same dataset in single-process vs multi-process mode, which is confusing and potentially problematic for reproducibility.
Changes Made
Testing
All changes have been thoroughly tested:
Backward Compatibility
This change is fully backward compatible.
By the way, I have also updated the README.md for introduction of this auto_size function. Right now, the create_patches.py behave exactly the same as before, with use_mp and auto_size stay disabled. Nonetheless, I think it would be better to enable the use_mp function in example.sh to improve consistency with the crop_image scripts.