refactor(inspect): Improve pipeline visualization and add max_epochs #3201
Conversation
Signed-off-by: Max Xiang <[email protected]>
Signed-off-by: Max Xiang <[email protected]>
There was a problem hiding this comment.
Pull request overview
This PR refactors the pipeline visualization system and adds configurable training epochs. The changes improve the anomaly detection visualization by simplifying the heatmap overlay logic and making the prediction labels more readable with better styling. Additionally, it introduces a max_epochs parameter to the training job payload, allowing users to configure the number of training epochs instead of using a hardcoded value.
Key Changes:
- Added
max_epochsparameter to training job payload with validation (minimum 1, default 200) - Refactored visualization overlay methods for cleaner code and improved visual styling
- Added
is_bgrflag to prediction pipeline to properly handle color space conversions
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| application/backend/src/pydantic_models/job.py | Added max_epochs field to TrainJobPayload with default value and validation |
| application/backend/src/services/training_service.py | Integrated max_epochs from job payload into training pipeline |
| application/backend/src/services/model_service.py | Added is_bgr parameter to handle color space conversion properly |
| application/backend/src/workers/inference.py | Passed is_bgr=True flag to prediction method |
| application/backend/src/utils/visualization.py | Refactored heatmap and label overlay methods with improved styling and cleaner logic |
| application/backend/tests/unit/services/test_training_service.py | Updated tests to pass max_epochs parameter |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: Max Xiang <[email protected]>
Signed-off-by: Max Xiang <[email protected]>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: Max Xiang <[email protected]>
ashwinvaidya17
left a comment
ashwinvaidya17
left a comment
There was a problem hiding this comment.
Looks good. Had a quick look at it so these are my initial comments.
| model_name: str | ||
| device: str | None = Field(default=None) | ||
| dataset_snapshot_id: str | None = Field(default=None) # used because UUID is not JSON serializable | ||
| max_epochs: int | None = Field(default=None, ge=1) |
There was a problem hiding this comment.
How are we planning on dealing with rest of the configurable parameters?
There was a problem hiding this comment.
We don't for the time being. We're likely going to remove this once we have a proper design for configurable parameters.
| return original_image | ||
|
|
||
| @staticmethod | ||
| def overlay_anomaly_heatmap( |
There was a problem hiding this comment.
Something for the future but maybe we should consolidate the visualization methods here and the ones in anomalib. This way we won't have to maintain two separate visualizers
Signed-off-by: Max Xiang <[email protected]>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: Max Xiang <[email protected]>
| model=model, | ||
| device=device, | ||
| synchronization_parameters=synchronization_parameters, | ||
| max_epochs=max_epochs, |
There was a problem hiding this comment.
Should we add this to the model training dialog? cc @ashwinvaidya17, @MarkRedeman
There was a problem hiding this comment.
It would be nice if we can already expose this (perhaps next to the picker of the training device from the new designs)
However there are some edge cases we would need to deal with. Some models will overwrite the max epochs to be 1 since they only need to do a single pass.
We could hardcode the UI to not show the input for those models, but that might get messy.
There was a problem hiding this comment.
Let's expose this and just have a note saying something like, "some models only extract features so max_epochs will be overridden for those models".
Signed-off-by: Colorado, Camilo <[email protected]>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 18 out of 18 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
application/ui/src/features/inspect/toolbar/pipeline-switch/pipeline-switch.test.tsx
Show resolved
Hide resolved
Signed-off-by: Max Xiang <[email protected]>
Signed-off-by: Max Xiang <[email protected]>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 18 out of 18 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
ashwinvaidya17
left a comment
ashwinvaidya17
left a comment
There was a problem hiding this comment.
Thanks for the efforts
maxxgx
merged commit e42e288
into
open-edge-platform:feature/geti-inspect
5 participants
📝 Description
max_epochsto job payloadScreen.Recording.2025-12-08.at.16.40.37.mov
Overlay ON/OFF
Screen.Recording.2025-12-09.at.13.21.56.mov
✨ Changes
Select what type of change your PR is:
✅ Checklist
Before you submit your pull request, please make sure you have completed the following steps:
For more information about code review checklists, see the Code Review Checklist.