Fix/tiling shm#7
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request focuses on optimizing the tiling preprocessing pipeline by refining resource allocation and introducing materialization steps to improve stability and performance. Additionally, it updates project dependencies to align with newer versions of key libraries, ensuring better compatibility and functionality across the data processing workflow. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request optimizes the tiling pipeline by introducing materialize() calls to manage memory and backpressure, updating resource allocations, and implementing conditional logic for epithelium overlap computation. It also updates core library dependencies and increases job resource requirements. Feedback was provided regarding the use of the logging module instead of print for better log management and utilizing Ray's add_column method for more efficient constant column addition.
| print("[INFO] Computing overlap: epithelium_overlap from epithelium_mask_path") | ||
| tiles = add_tile_overlap( | ||
| tiles, full_roi, "epithelium_mask_path", "epithelium_overlap", "0" | ||
| ) | ||
| else: | ||
| print( | ||
| "[INFO] Skipping epithelium overlap: dataset.mlflow_uris.epithelium_masks is not set" | ||
| ) |
There was a problem hiding this comment.
For better logging practices and consistency, it's recommended to use the logging module instead of print for informational messages. This allows for more flexible log level management and redirection.
You would need to add import logging at the top of the file. Then you can change the print statements to logging.info(...).
| tiles = tiles.map_batches( | ||
| add_missing_epithelium_overlap, | ||
| batch_format="pandas", | ||
| num_cpus=0.1, | ||
| memory=256 * 1024**2, | ||
| ) |
There was a problem hiding this comment.
Using map_batches just to add a column with a constant value is less efficient and idiomatic than using add_column. Ray Data's add_column is optimized for this use case and is more readable.
You can replace this map_batches call and the add_missing_epithelium_overlap function (lines 29-31) with a single line:
tiles = tiles.add_column("epithelium_overlap", lambda df: np.nan)This change would make the code more concise and performant.
tiles = tiles.add_column("epithelium_overlap", lambda df: np.nan)1 participant
No description provided.