Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Rename Classes UQL Operation #656

Merged
merged 11 commits into from
Sep 20, 2024

Conversation

NickHerrig
Copy link
Collaborator

Description

This PR adds UQL operation to handle renaming of classes given a class_map.

Type of change

Please delete options that are not relevant.

  • New feature (non-breaking change which adds functionality)

How has this change been tested, please provide a testcase or example of how you tested the change?

  • Integration tests passing
  • Local inferences server in Workflow UI

@NickHerrig
Copy link
Collaborator Author

@PawelPeczek-Roboflow let me know if you have any feedback any here from our convo today.

Copy link
Collaborator

@PawelPeczek-Roboflow PawelPeczek-Roboflow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is one issue with potential collapse of class id:
original detections have 1-1 relationship between class name and class id, after mapping this is not guaranteed.
Additional difficulty is also keeping original class.

Maintaining classes ids may be important - for instance to have visualisation that works consistently (look how this is implemented in supervision)

Proposed solution:

  • enforce full mapping - if not given raise operation error
  • when classes collapse - keep lower class id

if for some reason we wish to let non-strict class mappings, I suggest adding operation parameter (strict: bool with default True), such that people take risk by willingly declaring so.

@@ -28,6 +28,11 @@ def red_image() -> np.ndarray:
return cv2.imread(os.path.join(ASSETS_DIR, "red_image.png"))


@pytest.fixture(scope="function")
def fruit_image() -> np.ndarray:
return cv2.imread(os.path.join(ASSETS_DIR, "multi-fruit.jpg"))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please report image credits

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Complete

@@ -0,0 +1,124 @@
import numpy as np
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

great tests coverage

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@PawelPeczek-Roboflow Hey Pawel, I couldn't figure out or find a good example of passing Input Parameters into UQL operations for tests. Instead I created a hacky work around with parameterized tests to replace the Workflow Specification to test scenarios. Feel free to change this if you'd like; as I would also like to learn how to pass input parameters into UQL operations for future work.

@PawelPeczek-Roboflow
Copy link
Collaborator

@NickHerrig planning release tmrw - would u be able to apply changes? if not I can help, but let's agree on what we do with doubts I raised

@NickHerrig
Copy link
Collaborator Author

NickHerrig commented Sep 19, 2024

@PawelPeczek-Roboflow yes I have time now to complete these changes. I agree with your proposed solution to avoid same class names having different class-id(collapse) causing variability in annotation color, etc.

@PawelPeczek-Roboflow
Copy link
Collaborator

ok, great

@NickHerrig
Copy link
Collaborator Author

@PawelPeczek-Roboflow I implemented most of your propsal, with one slight addition to handle the case of assigning new class_ids when a class name does not exist in the original detections.

  • If strict mode is enabled (which is the default), the function will raise an OperationError if any class name in the original detections is not found in the class_map. This ensures that every class name in the detections must have a corresponding mapping in the class_map.

  • If strict mode is disabled (strict=False), the function will not raise an error for unmapped class names. Instead, it will use the original class name if it is not found in the class_map.

  • If a new class name already exists in the new_class_names list, the function reuses the corresponding class ID.

  • If a new class name does not exist in the original detections, the function assigns a new class ID by incrementing the maximum class ID found in the original detections. This ensures that new class IDs do not conflict with existing ones.

Feel free to make changes if you see a problem with this approach.

Copy link
Collaborator

@PawelPeczek-Roboflow PawelPeczek-Roboflow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added additional params to avoid collisions, tested and refactored the code to avoid O(n^2) complexity

@PawelPeczek-Roboflow
Copy link
Collaborator

@hansent @EmilyGavrilenko - this would need to be implemented on UI end: #656

@PawelPeczek-Roboflow PawelPeczek-Roboflow merged commit 9594505 into main Sep 20, 2024
58 checks passed
@PawelPeczek-Roboflow PawelPeczek-Roboflow deleted the nick/feat/uql-rename-classes branch September 20, 2024 08:57
@NickHerrig
Copy link
Collaborator Author

Thanks for your help @PawelPeczek-Roboflow getting this across the line!

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

Successfully merging this pull request may close these issues.

3 participants