Conversation
przemyslaw-aszkowski
left a comment
There was a problem hiding this comment.
Hi,
Thank you for your contribution!
I left a few comments for you to look at.
Moreover, the integration tests do not pass, please have a look at it: https://github.com/PUTvision/qgis-plugin-deepness/actions/runs/16948111454/job/48079565026?pr=221
Let me know if you need any help!
| Parameters for Inference of detection model (including pre/post-processing) obtained from UI. | ||
| """ | ||
|
|
||
| from deepness.processing.models.model_base import ModelBase |
There was a problem hiding this comment.
why import here instead of at the top of the file?
| qgisMinimumVersion=3.22 | ||
| description=Inference of deep neural network models (ONNX) for segmentation, detection and regression | ||
| version=0.7.0 | ||
| version=0.6.5 |
There was a problem hiding this comment.
Please increase the version number to 0.8.0
| filtered_bounding_boxes = [det for det in bounding_boxes if det.clss == channel_id] | ||
| print(f'Detections for class {channel_id}: {len(filtered_bounding_boxes)}') | ||
|
|
||
| vlayer = QgsVectorLayer("multipolygon", self.model.get_channel_name(0, channel_id), "memory") |
There was a problem hiding this comment.
Why was this code removed? Is it intended? I think it was added in a recent PR. Maybe there were some merge conflicts not resolved correctly?
| # import packages | ||
| from qgis.core import QgsTask, QgsApplication, QgsProject, QgsVectorLayer, QgsField, QgsWkbTypes | ||
| from qgis.PyQt.QtCore import QVariant | ||
| import torch |
There was a problem hiding this comment.
You can't use PyTorch in Deepness
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for RT-DETR model outputs in the deepness package, extending the existing detection capabilities to handle predictions from Ultralytics RT-DETR models. Additionally, it introduces a new chip classification feature for analyzing image patches within bounding boxes.
- Adds RT-DETR detector type with custom preprocessing and postprocessing logic
- Implements chip classification functionality with PyTorch model support
- Updates model base class to handle different preprocessing paths based on detector type
Reviewed Changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| src/deepness/processing/models/preprocessing_utils.py | Adds RT-DETR specific normalization functions and debug print statements |
| src/deepness/processing/models/model_base.py | Updates preprocessing logic to handle RT-DETR differently from other models |
| src/deepness/processing/models/detector.py | Implements RT-DETR postprocessing method with confidence filtering and NMS |
| src/deepness/processing/map_processor/map_processor_detection.py | Removes attribute setting code and adds debug parameters |
| src/deepness/processing/classify_chip.py | New file implementing chip classification task with PyTorch integration |
| src/deepness/metadata.txt | Version downgrade from 0.7.0 to 0.6.5 |
| src/deepness/deepness_dockwidget.py | Adds comprehensive GUI for chip classification configuration |
| src/deepness/deepness.py | Integrates chip classification task and result handling |
| src/deepness/common/processing_parameters/detection_parameters.py | Adds RT_DETR enum value and parameters |
| src/deepness/common/processing_parameters/classify_chip_parameters.py | New dataclass for chip classification parameters |
Comments suppressed due to low confidence (1)
src/deepness/deepness_dockwidget.py:272
- Missing space before parameter 'i'. Should be 'tile_params_batched: List[TileParams], i)'.
# insert the entire block just below "Training data export" block
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| :return: Batch of tiles with standardized values | ||
| """ | ||
| print(f'params.mean: {params.mean}') | ||
| print(f'params.std: {params.std}') |
There was a problem hiding this comment.
Debug print statements should be removed from production code. Use proper logging instead.
| print(f'params.std: {params.std}') | |
| logging.debug(f'params.mean: {params.mean}') | |
| logging.debug(f'params.std: {params.std}') |
| :return: Batch of tiles with standardized values | ||
| """ | ||
| print(f'params.mean: {params.mean}') | ||
| print(f'params.std: {params.std}') |
There was a problem hiding this comment.
Debug print statements should be removed from production code. Use proper logging instead.
| print(f'params.std: {params.std}') | |
| logging.debug(f'params.mean: {params.mean}') | |
| logging.debug(f'params.std: {params.std}') |
| # np.save(f"unprocessed_qgis_input{i}", tiles_batched) | ||
| input_batch = self.preprocessing(tiles_batched) | ||
| # print(f'preprocessed tiles_batched.shape: {input_batch.shape}') | ||
| # np.save(f"preprocessed_qgis_input{i}", input_batch) |
There was a problem hiding this comment.
Commented debug code should be removed from the codebase.
| # np.save(f"preprocessed_qgis_input{i}", input_batch) | |
| input_batch = self.preprocessing(tiles_batched) |
| """ | ||
| TILE_SIZE = tiles_batched.shape[1] | ||
| # print(f'unprocessed tiles_batched.shape: {tiles_batched.shape}') | ||
| # np.save(f"unprocessed_qgis_input{i}", tiles_batched) |
There was a problem hiding this comment.
Commented debug code should be removed from the codebase.
| # np.save(f"unprocessed_qgis_input{i}", tiles_batched) |
| # np.save(f"unprocessed_qgis_input{i}", tiles_batched) | ||
| input_batch = self.preprocessing(tiles_batched) | ||
| # print(f'preprocessed tiles_batched.shape: {input_batch.shape}') | ||
| # np.save(f"preprocessed_qgis_input{i}", input_batch) |
There was a problem hiding this comment.
Commented debug code should be removed from the codebase.
| # np.save(f"preprocessed_qgis_input{i}", input_batch) | |
| input_batch = self.preprocessing(tiles_batched) |
| # np.save(f"unprocessed_qgis_input{i}", tiles_batched) | ||
| input_batch = self.preprocessing(tiles_batched) | ||
| # print(f'preprocessed tiles_batched.shape: {input_batch.shape}') | ||
| # np.save(f"preprocessed_qgis_input{i}", input_batch) |
There was a problem hiding this comment.
Commented debug code should be removed from the codebase.
| # np.save(f"preprocessed_qgis_input{i}", input_batch) | |
| input_batch = self.preprocessing(tiles_batched) |
| return [], [], [] | ||
| outputs_x1y1x2y2 = self.xywh2xyxy(outputs_filtered) | ||
| boxes = outputs_x1y1x2y2[:,:4] | ||
| probabilities = outputs_x1y1x2y2[:, 4:][:,0] |
There was a problem hiding this comment.
The probability extraction is incorrect. This only takes the first probability column, but RT-DETR outputs should contain class probabilities. Use np.max(outputs_x1y1x2y2[:, 4:], axis=1) to get the maximum probability per detection.
| probabilities = outputs_x1y1x2y2[:, 4:][:,0] | |
| probabilities = np.max(outputs_x1y1x2y2[:, 4:], axis=1) | |
| classes = np.argmax(outputs_x1y1x2y2[:, 4:], axis=1) |
|
|
||
| boxes = np.array(boxes, dtype=int) | ||
| conf = probabilities | ||
| classes = np.argmax(np.expand_dims(conf, axis=1), axis=1) |
There was a problem hiding this comment.
This logic is incorrect. The conf variable contains single probability values, not class probabilities. Classes should be extracted from the original model output before filtering: classes = np.argmax(outputs_x1y1x2y2[:, 4:], axis=1).
| classes = np.argmax(np.expand_dims(conf, axis=1), axis=1) | |
| classes = np.argmax(outputs_x1y1x2y2[pick_indxs, 4:], axis=1) |
|
|
||
| bounding_boxes_in_tile_batched = self._process_tile(tile_img_batched, tile_params_batched) | ||
| bounding_boxes_in_tile_batched = self._process_tile(tile_img_batched, tile_params_batched, i) | ||
| i+=1 |
There was a problem hiding this comment.
Missing space around operator. Should be 'i += 1'.
| i+=1 | |
| i += 1 |
| qgisMinimumVersion=3.22 | ||
| description=Inference of deep neural network models (ONNX) for segmentation, detection and regression | ||
| version=0.7.0 | ||
| version=0.6.5 |
There was a problem hiding this comment.
Version appears to be downgraded from 0.7.0 to 0.6.5. This could indicate an error in version management or merge conflict.
| version=0.6.5 | |
| version=0.7.0 |
|
Closing due to no activity |
Support model predictions from https://docs.ultralytics.com/models/rtdetr/