-
Notifications
You must be signed in to change notification settings - Fork 131
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 Distance Measurement Block #731
Add Distance Measurement Block #731
Conversation
inference/core/workflows/core_steps/classical_cv/distance_measurement/v1.py
Outdated
Show resolved
Hide resolved
inference/core/workflows/core_steps/classical_cv/distance_measurement/v1.py
Outdated
Show resolved
Hide resolved
inference/core/workflows/core_steps/classical_cv/distance_measurement/v1.py
Outdated
Show resolved
Hide resolved
inference/core/workflows/core_steps/classical_cv/distance_measurement/v1.py
Outdated
Show resolved
Hide resolved
inference/core/workflows/core_steps/classical_cv/distance_measurement/v1.py
Outdated
Show resolved
Hide resolved
inference/core/workflows/core_steps/classical_cv/distance_measurement/v1.py
Outdated
Show resolved
Hide resolved
if not reference_bbox_1 or not reference_bbox_2: | ||
raise ValueError(f"Reference class '{object_1_class_name}' or '{object_2_class_name}' not found in predictions.") | ||
|
||
if has_overlap(reference_bbox_1, reference_bbox_2) or has_axis_overlap(reference_bbox_1, reference_bbox_2, reference_axis): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I find this if statement hard to understand - especially the latter part.
The root source of my confusion is the choice to measure distance along single dimension - why not to calculate distance in 2D?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I understand the confusion.
The goal is to measure the distance or gap between the closest edges of two bounding boxes on a 2D plane.
I'm adding an illustration to clarify this further:
has_overlap
: check if bboxes overlap.has_axis_overlap
: check if there isn't avertical
orhorizontal
gap between the bboxes.
inference/core/workflows/core_steps/classical_cv/distance_measurement/v1.py
Show resolved
Hide resolved
inference/core/workflows/core_steps/classical_cv/distance_measurement/v1.py
Outdated
Show resolved
Hide resolved
tests/workflows/unit_tests/core_steps/classical_cv/test_distance_measurement.py
Show resolved
Hide resolved
Also - for future contributions - could you use direct pushes to the repository to make CI run smoothly? |
Hey @PawelPeczek-Roboflow , thanks for reviewing my PR. I will address more requests tomorrow. |
Now that I have access, future contributions will be made this way. thanks! |
4e4147a
to
61f3947
Compare
@ediardo I have concerns that this block may not be easy to adopt by clients and may produce errors - for instance when model do not provide reference object. I do understand that the block solves a particular issue for the client, hence I am going to approve it, but in case of any future problems you will be in charge of solving them. |
Description
Measure the distance between two bounding boxes on a 2D plane using a camera positioned perpendicular to the plane. This method requires footage from this specific perspective, along with either a reference object of known dimensions placed in the same plane as the bounding boxes or a pixel-to-centimeter ratio that defines how many pixels correspond to one centimeter.
Key features include:
Type of change
Please delete options that are not relevant.
How has this change been tested, please provide a testcase or example of how you tested the change?
Created 10 unit tests to check various scenario, including different calibration methods, empty required inputs, incorrect value formats, and overlapping bounding boxes.
Docs
N/A